diff --git a/README.md b/README.md index a6bde84e..566f5632 100644 --- a/README.md +++ b/README.md @@ -96,7 +96,7 @@ python3 -c "import os, base64; print(base64.b64encode(os.urandom(32)).decode())" VIGIL_SIGNING_KEY_SEED=Hk3PtP...P0c= ``` -> ⚠️ **If you're pasting the compose stack into Portainer** (or any other UI that round-trips through YAML), set the value in the **Environment variables** tab — never inline it into the YAML. If you must inline it, **wrap it in double quotes** (`"Hk3...P0c="`) — the trailing `=` is base64 padding and unquoted YAML can strip or mangle it. A malformed seed produces `binascii.Error: Incorrect padding` and breaks every agent checkin in the fleet. +> **If you're pasting the compose stack into Portainer** (or any other UI that round-trips through YAML), set the value in the **Environment variables** tab — never inline it into the YAML. If you must inline it, **wrap it in double quotes** (`"Hk3...P0c="`) — the trailing `=` is base64 padding and unquoted YAML can strip or mangle it. A malformed seed produces `binascii.Error: Incorrect padding` and breaks every agent checkin in the fleet. **3. Verify the seed is good before bringing the stack up:** diff --git a/agent/tests/test_branches_agent.py b/agent/tests/test_branches_agent.py index 55c2b383..1c2114a0 100644 --- a/agent/tests/test_branches_agent.py +++ b/agent/tests/test_branches_agent.py @@ -313,7 +313,7 @@ def setUp(self): def test_features_include_branches(self): self.assertIn("branches", features.FEATURES) - self.assertEqual(features.FEATURES, ("relevant", "branches", "boost")) + self.assertEqual(features.FEATURES, ("relevant", "branches", "boost", "signed_v2", "stack_source")) def test_checkin_sends_branches(self): resp = MagicMock() @@ -321,7 +321,7 @@ def test_checkin_sends_branches(self): with patch.object(client.requests, "post", return_value=resp) as post: client.checkin(self.config, {}) payload = post.call_args.kwargs["json"] - self.assertEqual(payload["features"], ["relevant", "branches", "boost"]) + self.assertEqual(payload["features"], ["relevant", "branches", "boost", "signed_v2", "stack_source"]) if __name__ == "__main__": diff --git a/agent/tests/test_checkin_allowlist.py b/agent/tests/test_checkin_allowlist.py index 3439cb10..7d0d389e 100644 --- a/agent/tests/test_checkin_allowlist.py +++ b/agent/tests/test_checkin_allowlist.py @@ -54,6 +54,19 @@ def test_empty_allowlist_is_sent(self): client.checkin(cfg, metrics=[]) self.assertEqual(self.sent["allowlist"], []) + def test_payload_says_whether_it_runs_as_root(self): + """QA-08: a managed agent on the monitor-mode unit runs unprivileged, + and the server needs that fact to warn before a deploy.""" + with tempfile.TemporaryDirectory() as tmp: + cfg = AgentConfig(server_url="http://127.0.0.1:1", agent_token="t" * 40, + data_dir=Path(tmp), mode="managed") + with patch.object(client.os, "geteuid", return_value=0): + client.checkin(cfg, metrics=[]) + self.assertIs(self.sent["runs_as_root"], True) + with patch.object(client.os, "geteuid", return_value=1000): + client.checkin(cfg, metrics=[]) + self.assertIs(self.sent["runs_as_root"], False) + if __name__ == "__main__": unittest.main() diff --git a/agent/tests/test_compose_check.py b/agent/tests/test_compose_check.py new file mode 100644 index 00000000..a820f83d --- /dev/null +++ b/agent/tests/test_compose_check.py @@ -0,0 +1,274 @@ +"""The agent checks compose's *resolved* configuration before `up` (2026-10-08). +The server checks the text; compose re-reads it (variables, include/extends, +path and boolean normalisation), so the check that counts is on what will run.""" +import tests._safety_net # noqa: F401 — the guard, even when this file is run or imported on its own + +import json +import os +import sys +import tempfile +import unittest +import unittest.mock +from pathlib import Path +from unittest.mock import patch + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + +# isort: split +from vigil_agent import composecheck, executor # noqa: F401 — executor first, it imports stacks +from vigil_agent.actions import stacks + +WD = "/opt/vigil/stacks/web" + + +def _cfg(**svc): + return {"services": {"a": {"image": "x", **svc}}} + + +class ProblemsTests(unittest.TestCase): + def test_a_plain_service_is_fine(self): + self.assertEqual(composecheck.problems(_cfg(volumes=[{"type": "bind", "source": "/srv/data", "target": "/d"}]), WD), []) + + def test_resolved_escapes_are_found(self): + cases = { + "interpolated bind": _cfg(volumes=[{"type": "bind", "source": "/etc", "target": "/e"}]), + "double slash": _cfg(volumes=[{"type": "bind", "source": "//run", "target": "/r"}]), + "ancestor of socket": _cfg(volumes=[{"type": "bind", "source": "/var", "target": "/v"}]), + "privileged": _cfg(privileged=True), + "pid host": _cfg(pid="host"), + "cap": _cfg(cap_add=["CAP_SYS_ADMIN"]), + "device": _cfg(devices=[{"source": "/dev/sda", "target": "/dev/sda"}]), + "api socket": _cfg(use_api_socket=True), + "volumes_from container": _cfg(volumes_from=["container:portainer"]), + "build outside": _cfg(build={"context": "/"}), + "secret outside": {**_cfg(), "secrets": {"s": {"file": "/etc/shadow"}}}, + "driver_opts device": {**_cfg(), "volumes": {"v": {"driver_opts": {"device": "/etc"}}}}, + } + for label, cfg in cases.items(): + with self.subTest(label): + self.assertTrue(composecheck.problems(cfg, WD)) + + def test_podman_shapes_are_checked_too(self): + """podman-compose's `config` returns the file much as written, so the + short and string forms must be caught as well as docker's normal form.""" + cases = { + "short bind": _cfg(volumes=["/etc:/e"]), + "short relative escape": _cfg(volumes=["../../../../etc:/e"]), + "short home": _cfg(volumes=["~:/h"]), + "dict bind without type": _cfg(volumes=[{"source": "/root", "target": "/r"}]), + "odd volume type": _cfg(volumes=[{"type": "npipe", "source": "x", "target": "/x"}]), + "privileged string": _cfg(privileged="true"), + "privileged yes": _cfg(privileged="Yes"), + "cap string": _cfg(cap_add="SYS_ADMIN"), + "sysctl list": _cfg(sysctls=["kernel.core_pattern=|/tmp/x"]), + "device short": _cfg(devices=["/dev/sda:/dev/sda:rwm"]), + "device escape": _cfg(devices=["/dev/dri/../sda"]), + "unknown key": _cfg(storage_opt={"size": "1G"}), + "include left over": {**_cfg(), "include": ["/etc/other.yml"]}, + "network container": _cfg(network_mode="container:portainer"), + "seccomp profile": _cfg(security_opt=["seccomp=/tmp/allow-all.json"]), + "selinux spc": _cfg(security_opt=["label=type:spc_t"]), + "additional context": _cfg(build={"context": WD, "additional_contexts": {"x": "/etc"}}), + "additional context list": _cfg(build={"context": WD, "additional_contexts": ["x=/"]}), + "dockerfile outside": _cfg(build={"context": WD, "dockerfile": "/etc/shadow"}), + "env_file outside": _cfg(env_file=["/etc/shadow"]), + "env_file in home": _cfg(env_file=["~/.ssh/id_rsa"]), + "build in home": _cfg(build="~/src"), + "variable left in a bind": _cfg(volumes=["$$HOME:/h"]), + "variable privileged": _cfg(privileged="${P}"), + "relative device driver": {**_cfg(), "volumes": {"v": {"driver_opts": {"type": "none", "o": "bind", "device": "etc"}}}}, + } + for label, cfg in cases.items(): + with self.subTest(label): + self.assertTrue(composecheck.problems(cfg, WD), label) + + def test_ordinary_stacks_still_pass(self): + # docker's resolved JSON for a normal stack, captured on the test VM 2026-10-08 + cfg = {"name": "cc", "networks": {"default": {"name": "cc_default", "ipam": {}}}, + "services": {"a": {"cap_add": ["NET_ADMIN"], "command": None, "entrypoint": None, + "environment": {"A": "1"}, "image": "nginx", "networks": {"default": None}, + "ports": [{"mode": "ingress", "target": 80, "published": "8080", "protocol": "tcp"}], + "restart": "unless-stopped", "sysctls": {"net.core.somaxconn": "1024"}, + "volumes": [{"type": "bind", "source": WD + "/data", "target": "/d", + "bind": {"create_host_path": True}}, + {"type": "bind", "source": "/srv/x", "target": "/x", "read_only": True}, + {"type": "volume", "source": "named", "target": "/n", "volume": {}}]}, + "b": {"build": {"context": WD + "/sub", "dockerfile": "Dockerfile"}, + "depends_on": {"a": {"condition": "service_started", "required": True}}, + "devices": [{"source": "/dev/fuse", "target": "/dev/fuse", "permissions": "rwm"}], + "volumes_from": ["a"], "privileged": False, + "security_opt": ["no-new-privileges:true"]}}, + "volumes": {"named": {"name": "cc_named"}}} + self.assertEqual(composecheck.problems(cfg, WD), []) + self.assertEqual(composecheck.problems(_cfg(volumes=["./data:/d", "named:/n", "/anon"], + network_mode="host", sysctls=["net.ipv4.ip_forward=1"]), WD), []) + + def test_relative_paths_are_pinned_to_the_stack_folder(self): + """`up` reads the checked file from the agent's data dir; a relative + path left in it would resolve there (../ = the agent's own state).""" + cfg = composecheck.pin(_cfg(volumes=["./data:/d", {"type": "bind", "source": "../x", "target": "/x"}], + env_file=".env", build="./src", + ), WD) + svc = cfg["services"]["a"] + self.assertEqual(svc["volumes"][0], WD + "/data:/d") + self.assertEqual(svc["volumes"][1]["source"], "/opt/vigil/stacks/x") + self.assertEqual(svc["env_file"], [WD + "/.env"]) + self.assertEqual(svc["build"]["context"], WD + "/src") + + def test_a_variable_anywhere_that_matters_is_refused(self): + self.assertTrue(composecheck.problems(_cfg(tmpfs=["/run/${X}"]), WD)) + self.assertTrue(composecheck.problems(_cfg(networks={"${NET}": None}), WD)) + self.assertTrue(composecheck.problems(_cfg(ulimits={"nofile": "$N"}), WD)) + # where it can only reach inside the container, docker's $$ stays fine + self.assertEqual(composecheck.problems(_cfg(environment={"A": "x$$y"}, command=["sh", "-c", "echo $$HOME"], + labels={"l": "$$"}), WD), []) + + def test_a_bind_inside_a_writable_bind_is_refused(self): + """A container that can write a folder can swap a symlink into it + between the check and the mount.""" + self.assertTrue(composecheck.problems(_cfg(volumes=["./:/stack", "./data:/d"]), WD)) + self.assertTrue(composecheck.problems(_cfg(volumes=["./data:/d"]), WD, live_rw=(WD,))) + self.assertTrue(composecheck.problems(_cfg(volumes=["/srv/a/b:/d"]), WD, live_rw=("/srv/a",))) + # read-only, or the same folder, cannot plant anything + self.assertEqual(composecheck.problems(_cfg(volumes=["./:/stack:ro", "./data:/d"]), WD), []) + self.assertEqual(composecheck.problems(_cfg(volumes=["./data:/d"]), WD, live_rw=(WD + "/data",)), []) + + def test_a_symlink_already_in_the_stack_folder_is_refused(self): + with tempfile.TemporaryDirectory() as tmp: + os.mkdir(os.path.join(tmp, "real")) + os.symlink(os.path.join(tmp, "real"), os.path.join(tmp, "data")) + self.assertTrue(composecheck.problems(_cfg(volumes=["./data/sub:/d"]), tmp)) + + def test_a_symlink_planted_in_the_stack_folder_is_followed(self): + with tempfile.TemporaryDirectory() as tmp: + os.symlink("/etc", os.path.join(tmp, "data")) + self.assertTrue(composecheck.problems(_cfg(volumes=["./data:/d"]), tmp)) + self.assertTrue(composecheck.problems(_cfg(build={"context": tmp + "/data"}), tmp)) + + +class OwnershipTests(unittest.TestCase): + """A folder a non-root user can write lets that user swap a path + component for a symlink after the check — the engine, or the agent + writing as root, follows it.""" + + def test_a_bind_under_a_user_owned_folder_is_refused(self): + with tempfile.TemporaryDirectory() as tmp: # owned by the test user, under sticky /tmp + data = os.path.join(tmp, "data") + problems = composecheck.problems(_cfg(volumes=[f"{data}:/d"]), WD) + self.assertTrue(any("other than root" in p for p in problems), problems) + self.assertIsNone(composecheck.unsafe_dir("/srv/x/y")) # root-owned all the way (or not there yet) + + def test_the_stack_folder_itself_may_not_be_mounted_writable(self): + self.assertTrue(composecheck.problems(_cfg(volumes=["./:/stack"]), WD)) + self.assertTrue(composecheck.problems(_cfg(volumes=["/opt/vigil/stacks:/s"]), WD)) + self.assertTrue(composecheck.problems(_cfg(), WD, live_rw=(WD,))) + self.assertEqual(composecheck.problems(_cfg(volumes=["./:/stack:ro"]), WD), []) + + def test_x_podman_is_refused_anywhere(self): + for cfg in ({**_cfg(), "x-podman": {"in_pod": False}}, + _cfg(**{"x-podman": {"podman_args": ["--privileged"]}}), + _cfg(**{"X-Podman.uidmaps": ["0:0:1"]})): + with self.subTest(cfg=cfg): + self.assertTrue(composecheck.problems(cfg, WD)) + self.assertEqual(composecheck.problems(_cfg(**{"x-common": {"a": 1}}), WD), []) + + def test_agent_writes_never_follow_a_planted_link(self): + with tempfile.TemporaryDirectory() as tmp: + target = Path(tmp) / "elsewhere" + target.write_text("untouched") + link = Path(tmp) / "compose.yaml" + link.symlink_to(target) + stacks._write_new(link, "services: {}", 0o644) + self.assertFalse(link.is_symlink()) + self.assertEqual(link.read_text(), "services: {}") + self.assertEqual(target.read_text(), "untouched") + + def test_adopt_reads_never_follow_a_planted_link(self): + """_stack_read runs as root on paths from container labels, possibly in + a folder a non-root user owns: .env -> /etc/shadow must not be posted.""" + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + (root / "secret").write_text("root:$6$hash") + (root / "stack").mkdir() + (root / "stack" / "compose.yaml").write_text("services: {}") + self.assertEqual(stacks._read_plain(root / "stack" / "compose.yaml"), "services: {}") + self.assertIsNone(stacks._read_plain(root / "stack" / ".env")) + (root / "stack" / ".env").symlink_to(root / "secret") + with self.assertRaises(ValueError): + stacks._read_plain(root / "stack" / ".env") + (root / "linked").symlink_to(root / "stack") + with self.assertRaises(ValueError): + stacks._read_plain(root / "linked" / "compose.yaml") + with self.assertRaises(ValueError): + stacks._read_plain(root / "stack") # not a regular file + (root / "stack" / ".env").unlink() + os.link(root / "secret", root / "stack" / ".env") # a hard link is no symlink + with self.assertRaises(ValueError): + stacks._read_plain(root / "stack" / ".env") + + def test_hash_report_uses_private_copies_and_hides_compose_errors(self): + seen = [] + + def fake_run(cmd, timeout=60, extra_env=None): + seen.append(cmd) + raise RuntimeError("unexpected character in variable name 'root:$6$hash'") + with patch.object(stacks.ex, "_run", side_effect=fake_run), \ + patch.object(stacks.ex, "_compose_cmd", return_value=["docker", "compose"]), \ + patch.object(stacks.ex, "_compose_env", return_value={}): + report = stacks._hash_report("shop", Path("/srv/shop"), "services: {}", "A=1\n") + self.assertNotIn("root:", str(report)) + cmd = seen[0] + self.assertTrue(cmd[cmd.index("-f") + 1].startswith(tempfile.gettempdir())) + self.assertTrue(cmd[cmd.index("--env-file") + 1].startswith(tempfile.gettempdir())) + self.assertEqual(cmd[cmd.index("--project-directory") + 1], "/srv/shop") + + def test_deploy_refuses_a_user_owned_stack_folder(self): + with tempfile.TemporaryDirectory() as tmp: + with self.assertRaises(ValueError): + stacks._safe_workdir(Path(tmp) / "web") + os.symlink(tmp, Path(tmp) / "link") + with patch.object(composecheck, "unsafe_dir", return_value=None), self.assertRaises(ValueError): + stacks._safe_workdir(Path(tmp) / "link") + + +class DeployRefusesTests(unittest.TestCase): + def test_deploy_stops_before_up_when_the_resolved_config_escapes(self): + resolved = json.dumps(_cfg(volumes=[{"type": "bind", "source": "/etc", "target": "/e"}])) + calls = [] + + def fake_compose(workdir, project, *args, **kw): + calls.append(args) + return resolved if args[:1] == ("config",) else "up ok" + with tempfile.TemporaryDirectory() as tmp, \ + patch.object(stacks, "_compose", side_effect=fake_compose), \ + patch.object(stacks, "_live_rw_binds", return_value=()), \ + patch.object(stacks.client if hasattr(stacks, "client") else stacks, "fetch_stack_env", return_value="", create=True): + with self.assertRaises(ValueError): + stacks._check_resolved(Path(tmp), "web", "compose.yaml", Path(tmp)) + self.assertFalse([c for c in calls if c[:1] == ("up",)]) + + def test_live_writable_binds_come_from_vigil_stacks_only(self): + engine = unittest.mock.Mock() + engine.get.return_value = [ + {"Labels": {"com.docker.compose.project.config_files": "/data/stacks/web.json"}, + "Mounts": [{"Type": "bind", "Source": "/srv/web", "RW": True}, + {"Type": "bind", "Source": "/srv/ro", "RW": False}, + {"Type": "volume", "Source": "/var/lib/docker/volumes/x", "RW": True}]}, + {"Labels": {"com.docker.compose.project.config_files": "/home/admin/compose.yml"}, + "Mounts": [{"Type": "bind", "Source": "/home", "RW": True}]}, + {"Labels": {}, "Mounts": [{"Type": "bind", "Source": "/", "RW": True}]}, + ] + with patch.object(stacks.ex, "_engine", return_value=engine): + self.assertEqual(stacks._live_rw_binds(Path("/data")), ("/srv/web",)) + + def test_unreadable_config_refuses(self): + leak = "line 1: unexpected character in 'root:$6$hash'" + with patch.object(stacks, "_compose", side_effect=RuntimeError(leak)), \ + self.assertLogs("vigil_agent.actions.stacks", "WARNING"): + with self.assertRaises(ValueError) as caught: + stacks._check_resolved(Path("/tmp"), "web", "compose.yaml", Path("/tmp")) + self.assertNotIn("root:", str(caught.exception), "compose's error quotes host files: log it, don't return it") + + +if __name__ == "__main__": + unittest.main() diff --git a/agent/tests/test_compose_host_paths.py b/agent/tests/test_compose_host_paths.py new file mode 100644 index 00000000..dbab4391 --- /dev/null +++ b/agent/tests/test_compose_host_paths.py @@ -0,0 +1,210 @@ +"""QA-07: compose paths written by a manager that runs compose in its own +container (Portainer) are the container's, not the host's — map them through +the mount that holds them before compose or Vigil reads them.""" +import sys +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + +import tests._safety_net # noqa: F401 — the guard, even when this file is run or imported on its own + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + +# isort: split +from vigil_agent import engine, executor +from vigil_agent.actions import containers +from vigil_agent.config import AgentConfig + +from tests.fake_engine import FakeEngine + +_CFG = AgentConfig(server_url="https://v", agent_token="t", mode="full_control") +PORTAINER_COMPOSE = "/data/compose/49/docker-compose.yml" +PORTAINER_ENV = "/data/compose/49/stack.env" +PORTAINER_WD = "/data/compose/49" + + +def _mount(source: str, destination: str) -> dict: + return {"Type": "bind", "Source": source, "Destination": destination} + + +class HostPathTests(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + self.root = Path(self.tmp.name) + p = patch.object(executor, "_validate_path", side_effect=lambda v, label: Path(v)) + p.start() + self.addCleanup(p.stop) + + def _engine(self, containers_): + fake = FakeEngine(routes={("GET", "/containers/json"): (200, containers_)}) + self.addCleanup(fake.close) + p = patch.object(executor, "_engine", return_value=engine.EngineClient(fake.path)) + p.start() + self.addCleanup(p.stop) + return fake + + def test_existing_path_is_unchanged(self): + existing = self.root / "docker-compose.yml" + existing.write_text("services: {}\n") + fake = self._engine([{"Id": "p", "Mounts": [_mount(str(self.root / "host"), "/data")]}]) + self.assertEqual(containers._host_path(str(existing)), str(existing)) + self.assertFalse([r for r in fake.requests if "/containers/json" in r[1]]) + + def test_maps_through_the_containers_mount(self): + host = self.root / "host" / "compose" / "49" + host.mkdir(parents=True) + compose = host / "docker-compose.yml" + compose.write_text("services:\n searxng:\n image: searxng/searxng\n") + self._engine([{"Id": "p", "Mounts": [_mount(str(self.root / "host"), "/data"), + _mount("/var/run/docker.sock", "/var/run/docker.sock")]}]) + self.assertEqual(containers._host_path(PORTAINER_COMPOSE), str(compose)) + + def test_longest_destination_wins(self): + a = self.root / "a" / "compose" / "49" + b = self.root / "b" / "49" + a.mkdir(parents=True) + b.mkdir(parents=True) + (b / "docker-compose.yml").write_text("services: {}\n") + self._engine([{"Id": "p", "Mounts": [_mount(str(self.root / "b"), "/data/compose"), + _mount(str(self.root / "a"), "/data")]}]) + self.assertEqual(containers._host_path(PORTAINER_COMPOSE), str(b / "docker-compose.yml")) + + def test_unexplained_path_is_unchanged(self): + self._engine([{"Id": "p", "Mounts": [_mount(str(self.root / "elsewhere"), "/srv")]}]) + self.assertEqual(containers._host_path(PORTAINER_COMPOSE), PORTAINER_COMPOSE) + + def test_a_mount_that_maps_to_nothing_keeps_the_label_path(self): + self._engine([{"Id": "p", "Mounts": [_mount(str(self.root / "host"), "/data")]}]) + self.assertEqual(containers._host_path(PORTAINER_COMPOSE), PORTAINER_COMPOSE) + + def test_engine_error_is_unchanged(self): + """An agent with no answering engine keeps the label path — the + caller's error then names it. _mount_lookup swallows nothing else.""" + from vigil_agent.engine import EngineError + for boom in (RuntimeError("No container engine found (Docker or Podman socket)"), + EngineError("engine refused", status=500), + OSError("socket gone"), + ValueError("not json")): + p = patch.object(executor, "_engine", side_effect=boom) + p.start() + self.addCleanup(p.stop) + self.assertEqual(containers._host_path(PORTAINER_COMPOSE), PORTAINER_COMPOSE) + + def test_the_labelled_path_is_what_an_unexplained_failure_names(self): + """The fallback of last resort is the label itself, so an admin who + hits it sees the path Portainer's own stack entry shows.""" + self._engine([{"Id": "p", "Mounts": []}]) + self.assertEqual(containers._host_path("/data/compose/49/.env"), + "/data/compose/49/.env") + self.assertEqual(containers._label_env_files( + {"com.docker.compose.project.environment_file": "/data/compose/49/stack.env"}), []) + + +class PortainerActionTests(unittest.TestCase): + """The actions the cphmserv report hit, end to end through the seams: the + argv that reaches compose carries host paths, never a container's.""" + + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + self.host = Path(self.tmp.name) / "portainer_data" + stack = self.host / "compose" / "49" + stack.mkdir(parents=True) + (stack / "docker-compose.yml").write_text("services:\n searxng:\n image: searxng/searxng\n") + (stack / "stack.env").write_text("SEARXNG_SECRET=abc\n") + self.compose = stack / "docker-compose.yml" + self.env = stack / "stack.env" + self.fake = FakeEngine(routes={("GET", "/containers/json"): (200, [ + {"Id": "p", "Mounts": [_mount(str(self.host), "/data"), + _mount("/var/run/docker.sock", "/var/run/docker.sock")]}])}) + self.addCleanup(self.fake.close) + self.calls = [] + for name, value in (("_engine", engine.EngineClient(self.fake.path)), + ("_compose_cmd", ["docker", "compose"]), + ("_compose_env", {})): + p = patch.object(executor, name, return_value=value) + p.start() + self.addCleanup(p.stop) + p = patch.object(executor, "_run", side_effect=lambda cmd, timeout=60, extra_env=None: + self.calls.append(cmd) or "ok") + p.start() + self.addCleanup(p.stop) + p = patch.object(executor, "_validate_path", side_effect=lambda v, label: Path(v)) + p.start() + self.addCleanup(p.stop) + p = patch.object(executor.collector, "request_docker_recheck") + p.start() + self.addCleanup(p.stop) + + def test_portainer_container_rollback(self): + spec = {"Config": { + "Image": "searxng/searxng:latest", + "Labels": {"com.docker.compose.project": "searxng", + "com.docker.compose.service": "searxng", + "com.docker.compose.project.config_files": PORTAINER_COMPOSE, + "com.docker.compose.project.working_dir": PORTAINER_WD, + "com.docker.compose.project.environment_file": PORTAINER_ENV}}} + with patch.object(executor, "_docker_inspect", return_value=spec): + executor._container_rollback({"container_name": "searxng", + "image": "searxng/searxng:2025-09-17"}, _CFG) + + cmd = self.calls[0] + override = str(self.host / "compose" / "49" / containers.ROLLBACK_OVERRIDE) + self.assertEqual( + cmd, + ["docker", "compose", "-p", "searxng", "--project-directory", + str(self.host / "compose" / "49"), "--env-file", str(self.env), + "-f", str(self.compose), "-f", override, "up", "-d", "--no-deps", "searxng"]) + self.assertFalse([a for a in cmd if a.startswith("/data/")], + f"a container path leaked into the host's argv: {cmd!r}") + self.assertEqual((self.host / "compose" / "49" / containers.ROLLBACK_OVERRIDE).read_text(), + "# Written by Vigil: searxng rolled back. The next update removes it.\n" + "services:\n searxng:\n image: \"searxng/searxng:2025-09-17\"\n") + + def test_portainer_stack_compose_args(self): + self.fake.routes[("GET", "/containers/json")] = (200, [{ + "Id": "s", + "Labels": {"com.docker.compose.project": "searxng", + "com.docker.compose.project.config_files": PORTAINER_COMPOSE, + "com.docker.compose.project.working_dir": PORTAINER_WD, + "com.docker.compose.project.environment_file": PORTAINER_ENV}, + "Mounts": [_mount(str(self.host), "/data")]}]) + self.assertEqual( + containers._stack_compose_args("searxng"), + ["-p", "searxng", "--project-directory", str(self.host / "compose" / "49"), + "--env-file", str(self.env), "-f", str(self.compose)]) + + def test_portainer_update_container(self): + spec = {"Image": "sha256:old", "Config": { + "Image": "searxng/searxng:latest", + "Labels": {"com.docker.compose.project": "searxng", + "com.docker.compose.service": "searxng", + "com.docker.compose.project.config_files": PORTAINER_COMPOSE, + "com.docker.compose.project.working_dir": PORTAINER_WD, + "com.docker.compose.project.environment_file": PORTAINER_ENV}}} + with patch.object(executor, "_docker_inspect", return_value=spec), \ + patch.object(executor, "_pull", return_value=""), \ + patch.object(executor, "_image_id", return_value="sha256:new"), \ + patch.object(executor, "_container_image_id", return_value="sha256:new"): + executor._update_container({"container_name": "searxng"}, _CFG) + + self.assertEqual(len(self.calls), 2) + tail = ["--project-directory", str(self.host / "compose" / "49"), + "--env-file", str(self.env), "-f", str(self.compose)] + self.assertEqual(self.calls[0], ["docker", "compose", *tail, "pull", "searxng"]) + self.assertEqual(self.calls[1], ["docker", "compose", *tail, + "up", "-d", "--no-deps", "searxng"]) + for cmd in self.calls: # both compose runs carry the host's argv + self.assertFalse([a for a in cmd if a.startswith("/data/")], + f"a container path leaked into the host's argv: {cmd!r}") + # compose writes the labels from the paths it is handed, so the same + # re-up has to carry the env file — otherwise it interpolates blanks + # and recreates every service of the stack. + self.assertEqual(tail.count("--env-file"), 1) + self.assertEqual(tail[tail.index("--env-file") + 1], str(self.env)) + + +if __name__ == "__main__": + unittest.main() diff --git a/agent/tests/test_configured_server_key.py b/agent/tests/test_configured_server_key.py new file mode 100644 index 00000000..c1d3fb97 --- /dev/null +++ b/agent/tests/test_configured_server_key.py @@ -0,0 +1,71 @@ +"""SEC-3: the installer writes the server's public key into agent.yml, and then it +is the only key the agent trusts. No trust on first use, and a pin file in +data_dir (owned by the service account in monitor mode) cannot override it.""" +import tests._safety_net # noqa: F401 — the guard, even when this file is run or imported on its own + +import base64 +import sys +import tempfile +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + +# isort: split +from nacl.signing import SigningKey + +from vigil_agent import config as agent_config +from vigil_agent import verify + +REAL = base64.b64encode(bytes(SigningKey.generate().verify_key)).decode() +OTHER = base64.b64encode(bytes(SigningKey.generate().verify_key)).decode() + + +class ConfiguredKeyTests(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + self.data = Path(self.tmp.name) + + def test_the_configured_key_is_accepted(self): + key = verify.pin_public_key(self.data, REAL, configured=REAL) + self.assertEqual(base64.b64encode(bytes(key)).decode(), REAL) + + def test_a_different_key_from_the_server_is_refused(self): + with self.assertRaises(verify.KeyMismatchError): + verify.pin_public_key(self.data, OTHER, configured=REAL) + + def test_a_planted_pin_file_cannot_override_the_configured_key(self): + (self.data / "server_public_key.pin").write_text(OTHER) + key = verify.get_pinned_key(self.data, configured=REAL) + self.assertEqual(base64.b64encode(bytes(key)).decode(), REAL) + with self.assertRaises(verify.KeyMismatchError): + verify.pin_public_key(self.data, OTHER, configured=REAL) + + def test_without_a_configured_key_first_use_still_pins(self): + verify.pin_public_key(self.data, REAL) + with self.assertRaises(verify.KeyMismatchError): + verify.pin_public_key(self.data, OTHER) + + +class ConfigFieldTests(unittest.TestCase): + def _load(self, extra): + with tempfile.TemporaryDirectory() as tmp: + path = Path(tmp) / "agent.yml" + path.write_text(f'server_url: "https://v"\nagent_token: "t"\nmode: monitor\n{extra}') + path.chmod(0o600) + return agent_config.load_config(path) + + def test_key_is_read_from_agent_yml(self): + self.assertEqual(self._load(f'server_public_key: "{REAL}"\n').server_public_key, REAL) + + def test_a_malformed_key_is_refused(self): + with self.assertRaises(ValueError): + self._load('server_public_key: "not-a-key"\n') + + def test_absent_means_empty(self): + self.assertEqual(self._load("").server_public_key, "") + + +if __name__ == "__main__": + unittest.main() diff --git a/agent/tests/test_firewall_actions.py b/agent/tests/test_firewall_actions.py index 7f478519..2f1669bb 100644 --- a/agent/tests/test_firewall_actions.py +++ b/agent/tests/test_firewall_actions.py @@ -38,6 +38,61 @@ def snapshot(self): return SNAPSHOT +def _fw_cmd(params): + """The ufw command the removal action runs, with the tool calls faked.""" + seen = {} + + def fake_run(cmd, timeout=None): + seen["cmd"] = cmd + return "ok" + + with patch.object(firewall, "_run", fake_run), \ + patch.object(firewall.shutil, "which", lambda t: "/usr/sbin/ufw" + if t == "ufw" else None): + executor._remove_firewall_rule(params, _config()) + return seen["cmd"] + + +def _firewalld_cmds(params): + """Every firewall-cmd call the removal action runs.""" + calls = [] + + def fake_run(cmd, timeout=None): + calls.append(cmd) + return "ok" + + with patch.object(firewall, "_run", fake_run), \ + patch.object(firewall.shutil, "which", + lambda t: "/usr/bin/firewall-cmd" + if t == "firewall-cmd" else None): + executor._remove_firewall_rule(params, _config()) + return calls + + +class RemoveRejectRuleTests(unittest.TestCase): + """A reject rule (ufw reports REJECT) was removable by nobody: the agent + refused the action outright, so the only way back in was by hand.""" + + def test_a_reject_rule_is_removed_as_reject(self): + self.assertEqual( + _fw_cmd({"port": 22, "protocol": "tcp", "action": "reject"}), + ["ufw", "delete", "reject", "22/tcp"]) + + def test_firewalld_removes_a_reject_with_the_rich_rule(self): + calls = _firewalld_cmds({"port": 8080, "protocol": "tcp", + "action": "reject"}) + joined = [" ".join(c) for c in calls] + self.assertTrue( + any("--remove-rich-rule=" in c and "reject" in c for c in joined), + "a reject was written as a rich rule, so it must be removed as one") + self.assertFalse(any("--remove-port" in c for c in joined), + "a reject is not a port grant") + + def test_a_bogus_action_is_still_refused(self): + with self.assertRaises(ValueError): + _fw_cmd({"port": 22, "protocol": "tcp", "action": "drop"}) + + class ListRulesTests(unittest.TestCase): def test_the_snapshot_is_returned_as_json(self): with patch.object(firewall, "detect", lambda: FakeBackend()): diff --git a/agent/tests/test_privilege_mismatch_advice.py b/agent/tests/test_privilege_mismatch_advice.py index 1d6cc058..2906f6de 100644 --- a/agent/tests/test_privilege_mismatch_advice.py +++ b/agent/tests/test_privilege_mismatch_advice.py @@ -24,7 +24,7 @@ import unittest from unittest.mock import patch -from vigil_agent.__main__ import _warn_on_privilege_mismatch +from vigil_agent.__main__ import _privilege_mismatch, _process_tasks, _warn_on_privilege_mismatch class _Cfg: @@ -73,5 +73,32 @@ def test_the_advice_does_not_stop_at_removing_user(self): ) +class PrivilegeMismatchRejection(unittest.TestCase): + """QA-08: the warning is invisible from the UI, and the task it warns about + dies on '[Errno 30] Read-only file system' one Errno at a time.""" + + def test_managed_as_non_root_rejects_every_task(self): + tasks = [{"id": "1", "action": "update_agent"}, {"id": "2", "action": "check_service"}] + with patch("vigil_agent.__main__.os.geteuid", return_value=1000): + with patch("vigil_agent.__main__.execute_action") as run, \ + patch("vigil_agent.__main__._report_rejected") as rejected: + _process_tasks(tasks, _Cfg("managed"), None, object()) + self.assertEqual(rejected.call_count, 2, + "every task the agent cannot run must be rejected, not run") + run.assert_not_called() + for call in rejected.call_args_list: + message = call.args[2] + self.assertIn("install.sh", message, + "the only complete fix is re-running the installer") + self.assertIn("managed", message) + + def test_root_managed_is_not_rejected_for_privilege(self): + with patch("vigil_agent.__main__.os.geteuid", return_value=0): + self.assertFalse(_privilege_mismatch(_Cfg("managed"))) + with patch("vigil_agent.__main__.os.geteuid", return_value=1000): + self.assertFalse(_privilege_mismatch(_Cfg("monitor")), + "monitor mode needs no root — it must keep monitoring") + + if __name__ == "__main__": unittest.main() diff --git a/agent/tests/test_relevant_eval.py b/agent/tests/test_relevant_eval.py index 7461913f..dae4d619 100644 --- a/agent/tests/test_relevant_eval.py +++ b/agent/tests/test_relevant_eval.py @@ -248,7 +248,7 @@ def test_checkin_sends_features(self): with patch.object(client.requests, "post", return_value=resp) as post: client.checkin(self.config, {}) payload = post.call_args.kwargs["json"] - self.assertEqual(payload["features"], ["relevant", "branches", "boost"]) + self.assertEqual(payload["features"], ["relevant", "branches", "boost", "signed_v2", "stack_source"]) if __name__ == "__main__": unittest.main() diff --git a/agent/tests/test_script_inputs_env.py b/agent/tests/test_script_inputs_env.py index 9bbaeec5..4291ed9f 100644 --- a/agent/tests/test_script_inputs_env.py +++ b/agent/tests/test_script_inputs_env.py @@ -29,6 +29,7 @@ def _config(**kw): @unittest.skipUnless(os.name == "posix", "shell script") +@patch("vigil_agent.scripttrust.untrusted", new=lambda *_a: "") # temp dirs are not root's class ScriptInputEnvTests(unittest.TestCase): def _run_script(self, variables): with tempfile.TemporaryDirectory() as tmp: diff --git a/agent/tests/test_script_trust.py b/agent/tests/test_script_trust.py new file mode 100644 index 00000000..9e678fec --- /dev/null +++ b/agent/tests/test_script_trust.py @@ -0,0 +1,104 @@ +"""SEC-2: the agent runs a named script only if nobody but an administrator can change it. + +Reproduced live on the Windows test VM (2026-10-07): a standard user could create +C:\\ProgramData\\Vigil\\scripts, own it, and plant a script that execute_script then +ran as LocalSystem. Linux checked only the file's mode bits, not its owner or the +directories above it. +""" +import tests._safety_net # noqa: F401 — the guard, even when this file is run or imported on its own + +import sys +import unittest +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import patch + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + +# isort: split +from vigil_agent import scripttrust + +USERS = "S-1-5-32-545" +ALICE = "S-1-5-21-1111111111-2222222222-3333333333-1001" +SYSTEM = "S-1-5-18" +ADMINS = "S-1-5-32-544" +FULL = 0x1F01FF +READ_EXECUTE = 0x1200A9 + + +class FakePath: + def __init__(self, name, uid=0, mode=0o100755): + self.name, self._st = name, SimpleNamespace(st_uid=uid, st_mode=mode) + + def stat(self): + return self._st + + def __str__(self): + return self.name + + +class ChainTests(unittest.TestCase): + def test_chain_runs_from_the_script_to_the_scripts_dir(self): + root = Path("/etc/vigil/scripts") + self.assertEqual(scripttrust.chain(root / "a" / "b.sh", root), + [root / "a" / "b.sh", root / "a", root]) + + +class PosixTests(unittest.TestCase): + def test_root_owned_not_group_writable_is_fine(self): + self.assertEqual(scripttrust.posix_problem(FakePath("/s/x.sh")), "") + + def test_owned_by_someone_else_is_refused(self): + self.assertIn("not owned by root", scripttrust.posix_problem(FakePath("/s/x.sh", uid=1000))) + + def test_group_writable_is_refused(self): + self.assertIn("writable", scripttrust.posix_problem(FakePath("/s", mode=0o40775))) + + +class WindowsTests(unittest.TestCase): + def test_locked_folder_is_fine(self): + acl = {"owner": ADMINS, "rules": [{"sid": SYSTEM, "rights": FULL}, {"sid": ADMINS, "rights": FULL}, + {"sid": USERS, "rights": READ_EXECUTE}]} + self.assertEqual(scripttrust.windows_problem(Path("C:/x"), acl), "") + + def test_folder_a_standard_user_created_is_refused(self): + # What the VM showed: the creating user owns the folder with full control. + acl = {"owner": ALICE, "rules": [{"sid": ALICE, "rights": FULL}, {"sid": USERS, "rights": READ_EXECUTE}]} + self.assertIn("not an administrator", scripttrust.windows_problem(Path("C:/x"), acl)) + + def test_users_allowed_to_write_is_refused(self): + # ProgramData's inherited "Users: create files / write data". + acl = {"owner": ADMINS, "rules": [{"sid": USERS, "rights": 0x2 | 0x4}]} + self.assertIn(USERS, scripttrust.windows_problem(Path("C:/x"), acl)) + + def test_a_check_that_cannot_complete_refuses(self): + with patch.object(scripttrust, "os", SimpleNamespace(name="nt")), \ + patch.object(scripttrust, "windows_acl", side_effect=OSError("no powershell")): + problem = scripttrust.untrusted(Path("C:/p/scripts/x.ps1"), Path("C:/p/scripts")) + self.assertIn("could not check", problem) + + def test_untrusted_folder_above_a_trusted_file_is_refused(self): + good = {"owner": ADMINS, "rules": [{"sid": SYSTEM, "rights": FULL}]} + bad = {"owner": ALICE, "rules": [{"sid": ALICE, "rights": FULL}]} + acls = {"x.ps1": good, "scripts": bad} + with patch.object(scripttrust, "os", SimpleNamespace(name="nt")), \ + patch.object(scripttrust, "windows_acl", side_effect=lambda p: acls[Path(p).name]): + problem = scripttrust.untrusted(Path("C:/p/scripts/x.ps1"), Path("C:/p/scripts")) + self.assertIn("not an administrator", problem) + + +class RunTimeBackstopTests(unittest.TestCase): + """The installer cannot revoke a handle opened before its lock, so the + check that counts is the one execute_script makes every time it runs.""" + + def test_execute_script_checks_trust_on_every_run(self): + import inspect + + from vigil_agent import executor + src = inspect.getsource(executor._execute_script) + self.assertIn("untrusted(script_path, scripts_dir)", src) + self.assertLess(src.index("untrusted(script_path, scripts_dir)"), src.index("_run([str(script_path)]")) + + +if __name__ == "__main__": + unittest.main() diff --git a/agent/tests/test_signed_v2.py b/agent/tests/test_signed_v2.py new file mode 100644 index 00000000..561b9619 --- /dev/null +++ b/agent/tests/test_signed_v2.py @@ -0,0 +1,128 @@ +"""SEC-4: tasks are signed over when they were sent and which agent they were sent +to, and this agent refuses anything else. v1 left the dispatch time unsigned +(the TTL could be restarted by whoever relayed a task) and named a target host +that no agent ever checked.""" +import tests._safety_net # noqa: F401 — the guard, even when this file is run or imported on its own + +import base64 +import json +import sys +import tempfile +import unittest +from datetime import datetime, timedelta, timezone +from pathlib import Path +from unittest.mock import patch + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + +# isort: split +from nacl.signing import SigningKey + +from vigil_agent import __main__ as agent_main +from vigil_agent import nonce_store as ns +from vigil_agent import verify + +KEY = SigningKey.generate() +TOKEN = "this-agents-token" + + +def _now(delta=0): + return (datetime.now(timezone.utc) + timedelta(seconds=delta)).isoformat() + + +def _task(nonce="n1", dispatched_at=None, token=TOKEN, v2=True, ttl=300): + task = {"id": "t-" + nonce, "host_id": "h1", "action": "check_service", + "params": {"service_name": "cron"}, "nonce": nonce, "ttl_seconds": ttl, + "dispatched_at": dispatched_at or _now()} + fields = {k: task[k] for k in ("id", "host_id", "action", "params", "nonce", "ttl_seconds")} + if v2: + fields.update({"dispatched_at": task["dispatched_at"], "agent": verify.agent_fingerprint(token), "v": 2}) + task["sig_v"] = 2 + sig = KEY.sign(json.dumps(fields, sort_keys=True).encode()).signature + task["signature"] = base64.b64encode(sig).decode() + return task + + +class _Cfg: + mode = "full_control" + agent_token = TOKEN + + +class GateTests(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + self.store = ns.NonceStore(Path(self.tmp.name)) + self.rejected, self.ran = [], [] + for name, fn in (("_report_rejected", lambda c, t, r: self.rejected.append((t["id"], r))), + ("execute_action", lambda action, params, config: self.ran.append(action) or "ok"), + ("_report_completed", lambda *a, **k: None), + ("_report_failed", lambda *a, **k: None), + ("_privilege_mismatch", lambda c: False)): + p = patch.object(agent_main, name, fn) + p.start() + self.addCleanup(p.stop) + + def _process(self, task): + agent_main._process_tasks([task], _Cfg(), self.store, KEY.verify_key) + + def test_a_v2_task_for_this_agent_runs(self): + self._process(_task()) + self.assertEqual(self.ran, ["check_service"]) + self.assertEqual(self.rejected, []) + + def test_a_v1_signature_is_refused(self): + self._process(_task(v2=False)) + self.assertEqual(self.ran, []) + self.assertEqual(self.rejected[0][1], "Invalid signature") + + def test_restarting_the_ttl_breaks_the_signature(self): + task = _task(dispatched_at=_now(-3600)) + task["dispatched_at"] = _now() # a relay trying to make it fresh + self._process(task) + self.assertEqual(self.ran, []) + self.assertEqual(self.rejected[0][1], "Invalid signature") + + def test_a_task_signed_for_another_agent_is_refused(self): + self._process(_task(token="someone-elses-token")) + self.assertEqual(self.ran, []) + + def test_an_expired_task_is_refused(self): + self._process(_task(dispatched_at=_now(-3600), ttl=300)) + self.assertEqual(self.ran, []) + self.assertIn("expired", self.rejected[0][1]) + + def test_a_missing_dispatch_time_is_refused(self): + task = _task() + del task["dispatched_at"] + self._process(task) + self.assertEqual(self.ran, []) + + def test_a_replay_is_refused(self): + task = _task(nonce="again") + self._process(task) + self._process(dict(task)) + self.assertEqual(self.ran, ["check_service"]) + self.assertEqual(self.rejected[-1][1], "Replayed nonce") + + +class NonceRetentionTests(unittest.TestCase): + def test_a_nonce_is_kept_at_least_as_long_as_its_task_could_run(self): + with tempfile.TemporaryDirectory() as tmp: + store = ns.NonceStore(Path(tmp)) + store.record("long", ttl_seconds=7 * 86400) + with patch.object(ns.time, "time", return_value=__import__("time").time() + 6 * 86400): + store._prune() + self.assertTrue(store.seen("long")) + + def test_short_ttls_still_keep_an_hour(self): + with tempfile.TemporaryDirectory() as tmp: + store = ns.NonceStore(Path(tmp)) + store.record("short", ttl_seconds=60) + with patch.object(ns.time, "time", return_value=__import__("time").time() + 1800): + store._prune() + self.assertTrue(store.seen("short")) + + +if __name__ == "__main__": + unittest.main() diff --git a/agent/tests/test_stack_actions.py b/agent/tests/test_stack_actions.py index 8e9923d7..95bfb17a 100644 --- a/agent/tests/test_stack_actions.py +++ b/agent/tests/test_stack_actions.py @@ -48,7 +48,9 @@ def test_restart_uses_the_labels(self): "-f", "/opt/media/compose.yaml", "-f", "/opt/media/extra.yaml", "restart"]) self.assertEqual(env, {"DOCKER_HOST": f"unix://{self.fake.path}"}) self.assertEqual(out.data, {"project": "media"}) - query = self.fake.requests[-1][1] + # The mount lookup QA-07 adds is also a /containers/json call, so the + # filtered request this asserts on is found by name, not by position. + query = [q for _, q, _ in self.fake.requests if "filters=" in (q or "")][-1] self.assertIn("filters=", query) self.assertIn("com.docker.compose.project%3Dmedia", query) diff --git a/agent/tests/test_stack_deploy.py b/agent/tests/test_stack_deploy.py index bb49a922..ad97138a 100644 --- a/agent/tests/test_stack_deploy.py +++ b/agent/tests/test_stack_deploy.py @@ -1,4 +1,6 @@ """M11 09: stack_deploy writes the stack, fetches the .env once, ups it.""" +import dataclasses +import json import os import stat import sys @@ -49,17 +51,28 @@ def _params(self, **extra): "env_ticket": TICKET, "revision": 3, **extra} def test_deploy_writes_files_and_fetches_env_once(self): - with patch.object(client, "fetch_stack_env", return_value="API_KEY=hunter2\n") as fetch: - out = stacks._stack_deploy(self._params(), _CFG) - fetch.assert_called_once_with(_CFG, TICKET) + with patch.object(client, "fetch_stack_env", return_value="API_KEY=hunter2\n") as fetch, \ + patch.object(stacks, "_safe_workdir"), \ + patch.object(stacks, "_check_resolved", return_value={"services": {}}): # test_compose_check.py + cfg = dataclasses.replace(_CFG, data_dir=self.root / "agent-data") + out = stacks._stack_deploy(self._params(), cfg) + fetch.assert_called_once_with(cfg, TICKET) workdir = self.root / "media" self.assertEqual((workdir / "compose.yaml").read_text(), COMPOSE) env = workdir / ".env" self.assertEqual(env.read_text(), "API_KEY=hunter2\n") self.assertEqual(stat.S_IMODE(os.stat(env).st_mode), 0o600) - self.assertEqual(self.calls[0], ["docker", "compose", "-p", "media", "--project-directory", - str(workdir), "-f", str(workdir / "compose.yaml"), - "up", "-d", "--remove-orphans"]) + up = self.calls[0] + self.assertEqual(up[:5], ["docker", "compose", "-p", "media", "--project-directory"]) + self.assertEqual(up[5], str(workdir)) + self.assertEqual(up[-3:], ["up", "-d", "--remove-orphans"]) + # up reads the checked configuration, not compose.yaml again: kept in the agent's + # data dir (outside the stack folder, owner-only) for the containers' labels to name + checked = self.root / "agent-data" / "stacks" / "media.json" + self.assertEqual(up[6:8], ["-f", str(checked)]) + self.assertEqual(json.loads(checked.read_text()), {"services": {}}) + self.assertEqual(stat.S_IMODE(os.stat(checked).st_mode), 0o600) + self.assertEqual(stat.S_IMODE(os.stat(checked.parent).st_mode), 0o700) self.assertEqual(out.data, {"project": "media", "revision": 3}) self.assertNotIn("hunter2", str(out)) @@ -81,8 +94,12 @@ def test_remove_downs_and_only_deletes_vigils_own_folder(self): workdir = self.root / "media" workdir.mkdir(parents=True) (workdir / "compose.yaml").write_text(COMPOSE) - out = stacks._stack_remove({"project": "media", "working_dir": str(workdir), - "delete_files": True}, _CFG) + checked = self.root / "agent-data" / "stacks" / "media.json" + checked.parent.mkdir(parents=True) + checked.write_text("{}") + out = stacks._stack_remove({"project": "media", "working_dir": str(workdir), "delete_files": True}, + dataclasses.replace(_CFG, data_dir=self.root / "agent-data")) + self.assertFalse(checked.exists(), "the checked configuration goes with the stack") self.assertEqual(self.calls[0][-1], "down") self.assertEqual(out.data, {"project": "media", "files_deleted": True}) self.assertFalse(workdir.exists()) diff --git a/agent/tests/test_stack_read.py b/agent/tests/test_stack_read.py index 2ef13714..6f3781da 100644 --- a/agent/tests/test_stack_read.py +++ b/agent/tests/test_stack_read.py @@ -1,4 +1,5 @@ """M11 12: stack_read — adopt a stack in place, secrets off the task result.""" +import dataclasses import sys import tempfile import unittest @@ -31,6 +32,7 @@ def setUp(self): "com.docker.compose.config-hash": h, "com.docker.compose.project.config_files": str(self.dir / "docker-compose.yml"), "com.docker.compose.project.working_dir": str(self.dir)} + self.labels = labels("web", "h-web") self.fake = FakeEngine(routes={("GET", "/containers/json"): (200, [ {"Id": "a", "Labels": labels("web", "h-web")}, {"Id": "b", "Labels": labels("db", "h-old")}])}) self.addCleanup(self.fake.close) @@ -59,6 +61,18 @@ def test_reads_posts_and_keeps_secrets_out_of_the_result(self): self.assertEqual(out.data, {"project": "shop", "would_recreate": 1}) self.assertEqual(self.run_mock.call_args.args[0][-3:], ["config", "--hash", "*"]) + def test_reads_env_from_the_recorded_env_file(self): + """Portainer passes --env-file stack.env; adopt reads that, not .env.""" + (self.dir / "stack.env").write_text("A=1\n") + self.fake.routes[("GET", "/containers/json")] = (200, [ + {"Id": "a", "Labels": {**self.labels, + "com.docker.compose.project.environment_file": + str(self.dir / "stack.env")}}]) + (self.dir / ".env").unlink() + with patch.object(client, "post_stack_read") as post: + stacks._stack_read({"project": "shop", "adopt_ticket": TICKET}, _CFG) + self.assertEqual(post.call_args.args[2]["env"], "A=1\n") + def test_refusals(self): with patch.object(client, "post_stack_read") as post: for params in ({"project": "shop", "adopt_ticket": "nope"}, {"project": "Shop", "adopt_ticket": TICKET}): @@ -68,10 +82,13 @@ def test_refusals(self): def test_deploy_writes_an_adopted_stacks_own_file(self): with patch.object(client, "fetch_stack_env", return_value=""), \ - patch.object(executor.collector, "request_docker_recheck"): + patch.object(executor.collector, "request_docker_recheck"), \ + patch.object(stacks, "_safe_workdir"), \ + patch.object(stacks, "_check_resolved", return_value={"services": {}}) as check: stacks._stack_deploy({"project": "shop", "compose": "services:\n web:\n image: nginx\n", - "working_dir": str(self.dir), "compose_file": "docker-compose.yml"}, _CFG) - self.assertIn(str(self.dir / "docker-compose.yml"), self.run_mock.call_args.args[0]) + "working_dir": str(self.dir), "compose_file": "docker-compose.yml"}, + dataclasses.replace(_CFG, data_dir=self.dir / "agent-data")) + self.assertEqual(check.call_args.args[2], "docker-compose.yml") with self.assertRaises(ValueError): stacks._stack_deploy({"project": "shop", "compose": "services: {}", "working_dir": str(self.dir), "compose_file": "../etc/x.yml"}, _CFG) diff --git a/agent/tests/test_stack_source.py b/agent/tests/test_stack_source.py new file mode 100644 index 00000000..a7161c17 --- /dev/null +++ b/agent/tests/test_stack_source.py @@ -0,0 +1,176 @@ +"""A Git stack's source folder on the host (2026.14.1): the archive must be +the one the signed task names, may hold regular files only, and is written as +root without following any link a container or user planted in the folder.""" +import tests._safety_net # noqa: F401 — the guard, even when this file is run or imported on its own + +import dataclasses +import gzip +import hashlib +import io +import json +import os +import stat +import sys +import tarfile +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + +# isort: split +from vigil_agent import client, executor # noqa: F401 — executor first, it imports stacks +from vigil_agent.actions import stacks +from vigil_agent.config import AgentConfig + +TICKET = "6f1b2c3d-4e5f-4a6b-8c7d-9e0f1a2b3c4d" +COMPOSE = "services:\n app:\n build: ./app\n" + + +def _archive(entries) -> bytes: + """entries: (name, bytes|None for a dir, mode) or ("link", name, target).""" + raw = io.BytesIO() + with tarfile.open(fileobj=raw, mode="w") as tar: + for entry in entries: + if entry[0] == "link": + info = tarfile.TarInfo(entry[1]) + info.type, info.linkname = tarfile.SYMTYPE, entry[2] + tar.addfile(info) + continue + name, data, mode = entry + info = tarfile.TarInfo(name) + if data is None: + info.type = tarfile.DIRTYPE + tar.addfile(info) + else: + info.size, info.mode = len(data), mode + tar.addfile(info, io.BytesIO(data)) + return gzip.compress(raw.getvalue()) + + +GOOD = _archive([("compose.yaml", COMPOSE.encode(), 0o644), ("app", None, 0o755), + ("app/Dockerfile", b"FROM nginx\n", 0o644), ("run.sh", b"#!/bin/sh\n", 0o755)]) + + +def _sha(data): + return hashlib.sha256(data).hexdigest() + + +class SourceFilesTests(unittest.TestCase): + def test_the_archive_must_be_the_signed_one(self): + files = stacks._source_files(GOOD, _sha(GOOD)) + self.assertEqual(sorted(files), ["app/Dockerfile", "compose.yaml", "run.sh"]) + self.assertEqual(files["run.sh"][1], 0o755) + with self.assertRaisesRegex(ValueError, "does not match"): + stacks._source_files(GOOD, "0" * 64) + + def test_only_regular_files_at_plain_paths(self): + for bad in (_archive([("link", "etc", "/etc")]), _archive([("../x", b"x", 0o644)]), + _archive([("/abs", b"x", 0o644)]), _archive([("a//b", b"x", 0o644)])): + with self.subTest(), self.assertRaises(ValueError): + stacks._source_files(bad, _sha(bad)) + + +class PlaceSourceTests(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.root = Path(self.tmp.name) + self.work = self.root / "web" + self.work.mkdir() + self.manifest = self.root / "data" / "web.source.json" + self.outside = self.root / "outside" + self.outside.mkdir() + + def tearDown(self): + self.tmp.cleanup() + + def _place(self, archive=GOOD): + stacks._place_source(self.work, stacks._source_files(archive, _sha(archive)), self.manifest) + + def test_files_land_with_their_modes_and_a_private_manifest(self): + (self.work / "data").mkdir() + (self.work / "data" / "db.sqlite").write_text("keep me") + (self.work / ".env").write_text("A=1\n") + self._place() + self.assertEqual((self.work / "app" / "Dockerfile").read_text(), "FROM nginx\n") + self.assertEqual(stat.S_IMODE(os.stat(self.work / "run.sh").st_mode), 0o755) + self.assertEqual((self.work / "data" / "db.sqlite").read_text(), "keep me") + self.assertEqual((self.work / ".env").read_text(), "A=1\n") + self.assertEqual(json.loads(self.manifest.read_text()), ["app/Dockerfile", "compose.yaml", "run.sh"]) + self.assertEqual(stat.S_IMODE(os.stat(self.manifest).st_mode), 0o600) + + def test_a_planted_folder_link_is_refused_and_nothing_escapes(self): + os.symlink(self.outside, self.work / "app") + with self.assertRaisesRegex(ValueError, "not a plain folder"): + self._place() + self.assertEqual(list(self.outside.iterdir()), []) + + def test_a_planted_file_link_is_replaced_not_written_through(self): + target = self.outside / "cron" + target.write_text("untouched") + os.symlink(target, self.work / "run.sh") + self._place() + self.assertFalse((self.work / "run.sh").is_symlink()) + self.assertEqual(target.read_text(), "untouched") + + def test_files_a_newer_commit_dropped_are_removed_and_only_those(self): + self._place() + (self.work / "mine.txt").write_text("not from the repo") + smaller = _archive([("compose.yaml", COMPOSE.encode(), 0o644)]) + self._place(smaller) + self.assertFalse((self.work / "run.sh").exists()) + self.assertFalse((self.work / "app" / "Dockerfile").exists()) + self.assertTrue((self.work / "mine.txt").exists()) + + def test_a_forged_manifest_entry_cannot_reach_outside(self): + victim = self.outside / "victim" + victim.write_text("x") + self.manifest.parent.mkdir(parents=True) + self.manifest.write_text(json.dumps(["../outside/victim", "/etc/passwd"])) + os.symlink(self.outside, self.work / "lnk") + self.manifest.write_text(json.dumps(["../outside/victim", "lnk/victim"])) + self._place() + self.assertTrue(victim.exists()) + + +class DeployWithSourceTests(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.root = Path(self.tmp.name) + self.cfg = dataclasses.replace(AgentConfig(server_url="https://v", agent_token="t", mode="full_control"), + data_dir=self.root / "agent-data") + + def tearDown(self): + self.tmp.cleanup() + + def _deploy(self, archive, sha): + params = {"project": "web", "compose": COMPOSE, "working_dir": str(self.root / "web"), + "source_ticket": TICKET, "source_sha256": sha, "source_commit": "a" * 40} + with patch.object(client, "fetch_stack_source", return_value=archive) as fetch, \ + patch.object(stacks, "_safe_workdir"), \ + patch.object(stacks, "_check_resolved", return_value={"services": {}}), \ + patch.object(stacks, "_up", return_value="up"), \ + patch.object(executor.collector, "request_docker_recheck"): + stacks._stack_deploy(params, self.cfg) + fetch.assert_called_once_with(self.cfg, TICKET) + + def test_a_git_stack_gets_its_source_then_the_checked_compose(self): + self._deploy(GOOD, _sha(GOOD)) + self.assertEqual((self.root / "web" / "app" / "Dockerfile").read_text(), "FROM nginx\n") + self.assertEqual((self.root / "web" / "compose.yaml").read_text(), COMPOSE) + self.assertTrue((self.root / "agent-data" / "stacks" / "web.source.json").exists()) + + def test_a_tampered_archive_writes_nothing(self): + with self.assertRaisesRegex(ValueError, "does not match"): + self._deploy(GOOD, "0" * 64) + self.assertFalse((self.root / "web").exists()) + + def test_half_a_source_param_is_refused(self): + with self.assertRaises(ValueError): + stacks._source_params({"source_ticket": TICKET}) + self.assertIsNone(stacks._source_params({})) + + +if __name__ == "__main__": + unittest.main() diff --git a/agent/tests/test_update_container.py b/agent/tests/test_update_container.py index 121d16f1..5359d9ed 100644 --- a/agent/tests/test_update_container.py +++ b/agent/tests/test_update_container.py @@ -18,7 +18,7 @@ import tempfile import unittest from pathlib import Path -from unittest.mock import patch +from unittest.mock import MagicMock, patch sys.path.insert(0, str(Path(__file__).resolve().parents[1])) @@ -68,6 +68,8 @@ def fake_run(cmd, timeout=60, extra_env=None): stack.enter_context(patch.object(executor, "_container_image_id", return_value=after)) stack.enter_context(patch.object(executor, "_compose_cmd", return_value=["docker", "compose"])) stack.enter_context(patch.object(executor, "_compose_env", return_value={})) + stack.enter_context(patch.object( + executor, "_engine", return_value=MagicMock(get=MagicMock(return_value=[])))) mock = stack.enter_context(patch.object(executor, "_recreate_container", return_value="recreated")) if recreate else None yield mock diff --git a/agent/tests/test_windows_action_portability.py b/agent/tests/test_windows_action_portability.py index 20f8d786..17c9a989 100644 --- a/agent/tests/test_windows_action_portability.py +++ b/agent/tests/test_windows_action_portability.py @@ -84,7 +84,10 @@ def __init__(self, scripts_dir): self.scripts_dir = scripts_dir def test_a_script_inside_the_directory_is_accepted(self): - with patch("vigil_agent.executor._run", return_value="hi") as run: + # The temp dir is the test user's, not root's: ownership is + # tests.test_script_trust's subject, the path guard is this one's. + with patch("vigil_agent.scripttrust.untrusted", return_value=""), \ + patch("vigil_agent.executor._run", return_value="hi") as run: out = _execute_script({"script_name": "ok.sh"}, self._Cfg(self.dir)) self.assertEqual(out, "hi") run.assert_called_once() @@ -100,7 +103,8 @@ def test_a_world_writable_script_is_refused_on_posix(self): self.script.chmod(0o777) with self.assertRaises(ValueError) as ctx: _execute_script({"script_name": "ok.sh"}, self._Cfg(self.dir)) - self.assertIn("writable by group/others", str(ctx.exception)) + # Refused either way: the temp dir is not root's, and the file is 0777. + self.assertIn("Refusing to run ok.sh", str(ctx.exception)) class ScriptsDirDefault(unittest.TestCase): diff --git a/agent/vigil_agent/__main__.py b/agent/vigil_agent/__main__.py index 752604ea..7a8d72fd 100644 --- a/agent/vigil_agent/__main__.py +++ b/agent/vigil_agent/__main__.py @@ -61,6 +61,13 @@ def _process_tasks(tasks: list[dict], config, nonce_store: NonceStore, verify_ke ) return + if _privilege_mismatch(config): + # Better one clear rejection now than the task dying on Errno 30 + # halfway through a write (QA-08). + for task in tasks: + _report_rejected(config, task, PRIVILEGE_MISMATCH.format(mode=config.mode)) + return + if verify_key is None: if tasks: logger.warning( @@ -83,36 +90,36 @@ def _process_tasks(tasks: list[dict], config, nonce_store: NonceStore, verify_ke _report_rejected(config, task, "Replayed nonce") continue - # TTL check — bounded against when the SERVER dispatched the task, - # not when it was originally created. A task can sit in PENDING for - # hours (waiting on a schedule.window, retry delay, or offline host); - # the TTL only makes sense once the signed payload is on the wire. - # Falls back to ``created_at`` for compatibility with older servers. - ttl = task.get("ttl_seconds", 300) - ref_str = task.get("dispatched_at") or task.get("created_at") - if ref_str: - try: - ref = datetime.fromisoformat(ref_str) - if ref.tzinfo is None: - ref = ref.replace(tzinfo=timezone.utc) - if datetime.now(timezone.utc) > ref + timedelta(seconds=ttl): - logger.warning("Task %s has expired (TTL %ds) — rejecting", task_id, ttl) - _report_rejected(config, task, f"Task expired (TTL {ttl}s)") - nonce_store.record(nonce) - continue - except (ValueError, TypeError): - pass # malformed timestamp — skip the gate, signature still gates execution - - # Signature verification - if not verify.verify_task_signature(task, verify_key): + # Signature first (SEC-4): the dispatch time the TTL is measured from is + # inside the v2 signature, so it is only worth reading once verified. + if not verify.verify_task_signature(task, verify_key, config.agent_token): logger.warning("Task %s failed signature verification — rejecting", task_id) _report_rejected(config, task, "Invalid signature") - nonce_store.record(nonce) + continue + + # TTL — bounded against when the SERVER dispatched the task, not when it + # was created: a task can wait days for a schedule window. The time is + # signed, so it cannot be restarted by whoever relays the task, and a + # missing or unreadable one is refused rather than skipped. + ttl = task.get("ttl_seconds", 300) + try: + ref = datetime.fromisoformat(task["dispatched_at"]) + if ref.tzinfo is None: + ref = ref.replace(tzinfo=timezone.utc) + except (KeyError, ValueError, TypeError): + logger.warning("Task %s has no valid dispatch time — rejecting", task_id) + _report_rejected(config, task, "Task has no valid dispatch time") + nonce_store.record(nonce, ttl) + continue + if datetime.now(timezone.utc) > ref + timedelta(seconds=ttl): + logger.warning("Task %s has expired (TTL %ds) — rejecting", task_id, ttl) + _report_rejected(config, task, f"Task expired (TTL {ttl}s)") + nonce_store.record(nonce, ttl) continue # Execution — the agent validates each action against its own local # config. The server sends the script; we decide what's allowed. - nonce_store.record(nonce) + nonce_store.record(nonce, ttl) params = task.get("params", {}) if isinstance(params.get("steps"), list): @@ -578,6 +585,24 @@ def _report_skipped(config, task: dict, output: str, steps=None) -> None: logger.exception("Failed to report task %s skip", task.get("id")) +#: What a task gets back when this agent cannot run it. +PRIVILEGE_MISMATCH = ( + "This agent's mode ({mode}) runs tasks as root, but its service still runs it as the " + "unprivileged 'vigil-agent' user under the monitor-mode sandbox (read-only /usr and /etc), " + "so every task would fail. Re-run the installer on this host to regenerate the service for " + "its mode: curl -fsSL /agent/install.sh | sudo bash") + + +def _privilege_mismatch(config) -> bool: + """True when the mode needs root and this process is not root (POSIX only).""" + if config.mode == "monitor": + return False + try: + return os.geteuid() != 0 + except AttributeError: # Windows has no geteuid + return False + + def _warn_on_privilege_mismatch(config) -> None: """Say so when the mode needs root and this process does not have it. @@ -604,12 +629,7 @@ def _warn_on_privilege_mismatch(config) -> None: A warning, not a refusal: an agent that stops monitoring because it cannot execute is worse than one that monitors and says it cannot execute. """ - if config.mode == "monitor": - return - try: - if os.geteuid() == 0: - return - except AttributeError: # Windows has no geteuid + if not _privilege_mismatch(config): return logger.warning( @@ -733,7 +753,7 @@ def run_agent() -> None: signal.signal(signal.SIGTERM, _handle_signal) signal.signal(signal.SIGINT, _handle_signal) - verify_key = verify.get_pinned_key(config.data_dir) + verify_key = verify.get_pinned_key(config.data_dir, config.server_public_key) # ── Main checkin loop ──────────────────────────────────────────────── # Hardware inventory shifts at human timescales — refresh once per hour @@ -839,7 +859,8 @@ def run_agent() -> None: pub_key_b64 = response.get("public_key") if pub_key_b64: try: - verify_key = verify.pin_public_key(config.data_dir, pub_key_b64) + verify_key = verify.pin_public_key(config.data_dir, pub_key_b64, + config.server_public_key) except KeyMismatchError: logger.critical( "SERVER PUBLIC KEY HAS CHANGED. This could indicate a compromised server. " diff --git a/agent/vigil_agent/actions/containers.py b/agent/vigil_agent/actions/containers.py index cf1345ab..44e85c36 100644 --- a/agent/vigil_agent/actions/containers.py +++ b/agent/vigil_agent/actions/containers.py @@ -28,6 +28,22 @@ def _engine(): return ex._engine() +def _mount_lookup(): + """Every running container, for the mount lookup ``_host_path`` needs. + Unfiltered: the container holding the mount (Portainer itself) carries no + label of this project, so a project filter could never find it. None when + there is no engine to ask — the caller's error then names the label path, + which is what the admin can match. One lookup serves every path of an + action; ``_host_path`` and ``_label_env_files`` take it as ``known``.""" + try: + return _engine().get("/containers/json") or [] + except (RuntimeError, OSError, ValueError): + # no engine at all, or one that won't answer (EngineError is a + # RuntimeError): no mapping. The caller's error still names the + # labelled path, which is what the admin can see. + return None + + def _lifecycle(params: dict, verb: str) -> ActionOutput: name = _validate_name( params.get("container_name") or params.get("container_id", ""), @@ -368,15 +384,16 @@ def _update_container(params: dict, _config: AgentConfig) -> str: f"compose labels are missing config files — use " f"docker_compose_up with an explicit compose_file" ) - compose_file = str(ex._validate_path(compose_file, "compose_file")) + known = _mount_lookup() + compose_file = str(ex._validate_path(_host_path(compose_file, known), "compose_file")) - project_dir = labels.get("com.docker.compose.project.working_dir") or "" + project_dir = _host_path(labels.get("com.docker.compose.project.working_dir") or "", known) dir_args = [] if project_dir: dir_args = ["--project-directory", str(ex._validate_path(project_dir, "project directory"))] _clear_rollback(project_dir) # an update ends a rollback - cmd = [*ex._compose_cmd(), *dir_args, "-f", compose_file] + cmd = [*ex._compose_cmd(), *dir_args, *_label_env_files(labels, known), "-f", compose_file] ex._run(cmd + ["pull", service], timeout=600, extra_env=ex._compose_env()) ex._run(cmd + ["up", "-d", "--no-deps", service], timeout=300, extra_env=ex._compose_env()) @@ -480,16 +497,64 @@ def _stack_compose_args(project: str) -> list[str]: if not files: continue args = ["-p", project] - workdir = labels.get("com.docker.compose.project.working_dir") or "" + # One engine lookup serves the workdir, the env files and the compose files. + known = _mount_lookup() + workdir = _host_path(labels.get("com.docker.compose.project.working_dir") or "", known) if workdir: args += ["--project-directory", str(ex._validate_path(workdir, "project directory"))] + args += _label_env_files(labels, known) for f in files: - args += ["-f", str(ex._validate_path(f, "compose file"))] + args += ["-f", str(ex._validate_path(_host_path(f, known), "compose file"))] return args raise ValueError(f"No containers of stack {project!r} carry compose file labels — " f"deploy it with docker_compose_up and an explicit compose_file") +def _host_path(raw: str, known=None) -> str: + """The host's path for a path compose wrote into a label. + + A stack started by a manager that runs compose in its own container + (Portainer: /data/compose//docker-compose.yml) carries that container's + paths. Map them through the mount that holds them; a path that already + exists, or that no mount explains, comes back unchanged. ``known`` is the + caller's ``_mount_lookup()``, so one action asks the engine once. + """ + if not raw or Path(raw).exists(): + return raw + target = Path(raw) + if not target.is_absolute() or ".." in target.parts: + # a relative label can be read as escaping the mount it maps through; + # leave it to the caller, whose error names the labelled path + return raw + if known is None: + known = _mount_lookup() + if known is None: + return raw + best = None + for c in known: + for m in c.get("Mounts") or []: + dest, src = str(m.get("Destination") or ""), str(m.get("Source") or "") + if not dest.startswith("/") or not src: + continue + d = Path(dest) + if (target == d or d in target.parents) and (best is None or len(d.parts) > len(best[0].parts)): + best = (d, Path(src)) + if best is None: + return raw + mapped = best[1] / target.relative_to(best[0]) + return str(mapped) if mapped.exists() else raw + + +def _label_env_files(labels: dict, known=None) -> list[str]: + """``--env-file`` args for the env files compose recorded, mapped to the host.""" + args = [] + for f in str(labels.get("com.docker.compose.project.environment_file") or "").split(","): + f = _host_path(f.strip(), known) + if f and Path(f).is_file(): + args += ["--env-file", str(ex._validate_path(f, "env file"))] + return args + + def _stack_restart(params: dict, _config: AgentConfig) -> str: project = str(params.get("project") or "") args = _stack_compose_args(project) @@ -548,7 +613,8 @@ def _container_rollback(params: dict, _config: AgentConfig) -> str: if project: service = labels.get("com.docker.compose.service") or name _validate_name(service, "service name") - workdir = labels.get("com.docker.compose.project.working_dir") or "" + known = _mount_lookup() + workdir = _host_path(labels.get("com.docker.compose.project.working_dir") or "", known) files = [f.strip() for f in str(labels.get("com.docker.compose.project.config_files") or "") .split(",") if f.strip() and not f.strip().endswith(ROLLBACK_OVERRIDE)] if not files or not workdir: @@ -557,9 +623,10 @@ def _container_rollback(params: dict, _config: AgentConfig) -> str: override = wd / ROLLBACK_OVERRIDE override.write_text(f"# Written by Vigil: {service} rolled back. The next update removes it.\n" f"services:\n {service}:\n image: {json.dumps(image)}\n") - cmd = [*ex._compose_cmd(), "-p", project, "--project-directory", str(wd)] + cmd = [*ex._compose_cmd(), "-p", project, "--project-directory", str(wd), + *_label_env_files(labels, known)] for f in files: - cmd += ["-f", str(ex._validate_path(f, "compose file"))] + cmd += ["-f", str(ex._validate_path(_host_path(f, known), "compose file"))] cmd += ["-f", str(override), "up", "-d", "--no-deps", service] ex._run(cmd, timeout=300, extra_env=ex._compose_env()) via = "compose override" diff --git a/agent/vigil_agent/actions/stacks.py b/agent/vigil_agent/actions/stacks.py index 680850b7..7a218e3d 100644 --- a/agent/vigil_agent/actions/stacks.py +++ b/agent/vigil_agent/actions/stacks.py @@ -8,9 +8,11 @@ from __future__ import annotations +import logging import os import re import shutil +import stat from pathlib import Path from .. import collector @@ -18,6 +20,8 @@ from ..config import AgentConfig from ..executor import ActionOutput +log = logging.getLogger(__name__) + STACKS_ROOT = Path("/opt/vigil/stacks") COMPOSE_NAME = "compose.yaml" _PROJECT = re.compile(r"^[a-z0-9][a-z0-9_-]{0,62}$") @@ -39,14 +43,204 @@ def _workdir(params: dict) -> Path: return path -def _write_private(path: Path, text: str) -> None: - """Create or replace *path* readable by its owner only.""" - fd = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) +def _write_new(path: Path, text: str, mode: int) -> None: + """Replace *path* with a fresh file, never following a link: whatever is + there (a symlink a container planted, say) is removed, not written + through, and O_EXCL fails if one reappears in between.""" + try: + os.unlink(path) + except FileNotFoundError: + pass + fd = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_EXCL | getattr(os, "O_NOFOLLOW", 0), mode) try: os.write(fd, text.encode()) + os.fchmod(fd, mode) finally: os.close(fd) - os.chmod(path, 0o600) + + +MAX_READ = 1_000_000 + + +def _read_plain(path: Path) -> str | None: + """A regular file's text, read as root without following a symlink + anywhere on the way (each folder opened relative to the last, with + O_NOFOLLOW). The paths come from container labels and may sit in a folder a + non-root user owns, who could otherwise point .env at /etc/shadow and + have it posted to the server. None when there is no such file.""" + path = Path(path) + if not path.is_absolute() or ".." in path.parts: + raise ValueError(f"{path} must be an absolute path without '..'") + nofollow = getattr(os, "O_NOFOLLOW", 0) + fd = os.open("/", os.O_RDONLY | getattr(os, "O_DIRECTORY", 0)) + try: + for part in path.parts[1:-1]: + try: + nxt = os.open(part, os.O_RDONLY | getattr(os, "O_DIRECTORY", 0) | nofollow, dir_fd=fd) + except FileNotFoundError: + return None + except OSError as exc: # ELOOP / ENOTDIR: a symlink or not a folder + raise ValueError(f"refusing to read {path}: {part} is not a plain folder") from exc + os.close(fd) + fd = nxt + try: + leaf = os.open(path.name, os.O_RDONLY | nofollow | getattr(os, "O_NONBLOCK", 0), dir_fd=fd) + except FileNotFoundError: + return None + except OSError as exc: + raise ValueError(f"refusing to read {path}: it is a symlink or unreadable") from exc + try: + st = os.fstat(leaf) + if not stat.S_ISREG(st.st_mode): + raise ValueError(f"refusing to read {path}: not a regular file") + if st.st_nlink > 1: + # a hard link to /etc/shadow is no symlink, and needs no + # O_NOFOLLOW to get through (where fs.protected_hardlinks is off) + raise ValueError(f"refusing to read {path}: it has {st.st_nlink} hard links") + if st.st_size > MAX_READ: + raise ValueError(f"refusing to read {path}: larger than {MAX_READ} bytes") + with os.fdopen(leaf, "rb", closefd=False) as fh: + return fh.read(MAX_READ + 1).decode() + finally: + os.close(leaf) + finally: + os.close(fd) + + +_SHA256 = re.compile(r"^[0-9a-f]{64}$") +MAX_SOURCE_TREE = 100 * 1024 * 1024 +MAX_SOURCE_FILES = 5000 + + +def _source_files(archive: bytes, sha256: str) -> dict[str, tuple[bytes, int]]: + """A Git stack's source folder, from the archive the server packed: the + hash must be the one in the signed task, and it may hold regular files + only, at plain relative paths. {path: (bytes, mode)}.""" + import gzip + import hashlib + import io + import tarfile + + if hashlib.sha256(archive).hexdigest() != sha256: + raise ValueError("the stack's source does not match the signed task — refusing it") + files: dict[str, tuple[bytes, int]] = {} + total = 0 + with tarfile.open(fileobj=io.BytesIO(gzip.decompress(archive)), mode="r:") as tar: + for member in tar: + parts = member.name.split("/") + if (member.name.startswith("/") or "\\" in member.name + or any(p in ("", ".", "..") for p in parts)): + raise ValueError(f"the stack's source has an unusable path: {member.name!r}") + if member.isdir(): + continue + if not member.isreg(): + raise ValueError(f"the stack's source holds a non-regular file: {member.name!r}") + total += member.size + if len(files) >= MAX_SOURCE_FILES or total > MAX_SOURCE_TREE: + raise ValueError("the stack's source is too large") + files[member.name] = (tar.extractfile(member).read(), 0o755 if member.mode & 0o111 else 0o644) + return files + + +def _open_dir(parent_fd: int, name: str, *, create: bool) -> int: + """A folder under *parent_fd*, never through a symlink.""" + if create: + try: + os.mkdir(name, 0o755, dir_fd=parent_fd) + except FileExistsError: + pass + try: + return os.open(name, os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW, dir_fd=parent_fd) + except OSError as exc: # ELOOP / ENOTDIR: a symlink or a file where a folder belongs + raise ValueError(f"refusing to place the stack's source: {name} is not a plain folder") from exc + + +def _place_source(workdir: Path, files: dict[str, tuple[bytes, int]], manifest: Path) -> None: + """Write the source folder into the stack folder as root without following + a link anywhere: each folder is opened relative to the last with + O_NOFOLLOW, each file replaced (unlink + O_EXCL|O_NOFOLLOW). Files a + previous deploy placed and this source no longer has are removed — known + from a manifest in the agent's own data dir, which no container can + reach — and nothing else in the folder (data, the .env) is touched.""" + import json + + root_fd = os.open(workdir, os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW) + try: + for rel, (data, mode) in sorted(files.items()): + *folders, name = rel.split("/") + fd = os.dup(root_fd) + try: + for folder in folders: + nxt = _open_dir(fd, folder, create=True) + os.close(fd) + fd = nxt + try: + os.unlink(name, dir_fd=fd) + except FileNotFoundError: + pass + except IsADirectoryError as exc: + raise ValueError(f"refusing to place {rel}: a folder of that name is in the way") from exc + out = os.open(name, os.O_WRONLY | os.O_CREAT | os.O_EXCL | os.O_NOFOLLOW, mode, dir_fd=fd) + try: + os.write(out, data) + os.fchmod(out, mode) + finally: + os.close(out) + finally: + os.close(fd) + + try: + previous = set(json.loads(manifest.read_text())) + except (OSError, ValueError): + previous = set() + for rel in sorted(previous - set(files)): + *folders, name = str(rel).split("/") + if any(p in ("", ".", "..") for p in [*folders, name]): + continue + fd = os.dup(root_fd) + try: + for folder in folders: + nxt = _open_dir(fd, folder, create=False) + os.close(fd) + fd = nxt + st = os.stat(name, dir_fd=fd, follow_symlinks=False) + if stat.S_ISREG(st.st_mode): + os.unlink(name, dir_fd=fd) + except (OSError, ValueError): + pass # already gone, or no longer ours to remove + finally: + os.close(fd) + finally: + os.close(root_fd) + manifest.parent.mkdir(mode=0o700, parents=True, exist_ok=True) + _write_private(manifest, json.dumps(sorted(files))) + + +def _source_params(params: dict) -> tuple[str, str] | None: + """(ticket, sha256) for a Git stack's deploy, or None for a stack edited in Vigil.""" + ticket = str(params.get("source_ticket") or "") + sha = str(params.get("source_sha256") or "") + if not ticket and not sha: + return None + if not _TICKET.match(ticket) or not _SHA256.match(sha): + raise ValueError("source_ticket must be a ticket id and source_sha256 64 hex characters") + return ticket, sha + + +def _write_private(path: Path, text: str) -> None: + """Create or replace *path* readable by its owner only.""" + _write_new(path, text, 0o600) + + +def _safe_workdir(workdir: Path) -> None: + """The agent writes here as root, so no one but root may be able to + re-point it: every folder above it root-owned and not group/other-writable.""" + from ..composecheck import unsafe_dir + loose = unsafe_dir(str(workdir / "x")) + if loose: + raise ValueError(f"refusing to deploy into {workdir}: {loose} can be changed by a user other than root") + if workdir.is_symlink(): + raise ValueError(f"refusing to deploy into {workdir}: it is a symlink") _FILE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,99}\.ya?ml$") @@ -68,6 +262,77 @@ def _compose(workdir: Path, project: str, *args: str, timeout: int = 600, timeout=timeout, extra_env=ex._compose_env()) +def _resolved_config(workdir: Path, project: str, compose_file: str) -> dict: + """Compose's own resolved configuration: variables substituted from the + .env, include/extends merged, paths and booleans normalised.""" + import json + + import yaml + try: + return json.loads(_compose(workdir, project, "config", "--format", "json", + timeout=120, compose_file=compose_file)) + except (RuntimeError, ValueError): + # podman-compose has no --format json; its plain output is YAML. + doc = yaml.safe_load(_compose(workdir, project, "config", timeout=120, compose_file=compose_file)) + if not isinstance(doc, dict): + raise ValueError("compose config did not produce a configuration") from None + return doc + + +def _live_rw_binds(data_dir: Path) -> tuple: + """Host paths that running containers of Vigil-deployed stacks (those + started from a checked file in /stacks) mount writable.""" + checked = str(Path(data_dir) / "stacks") + "/" + out = [] + for c in ex._engine().get("/containers/json", query={"all": "1"}) or []: + files = str((c.get("Labels") or {}).get("com.docker.compose.project.config_files") or "") + if not any(f.strip().startswith(checked) for f in files.split(",")): + continue + out += [m.get("Source") for m in c.get("Mounts") or [] + if m.get("Type") == "bind" and m.get("RW") and m.get("Source")] + return tuple(out) + + +def _check_resolved(workdir: Path, project: str, compose_file: str, data_dir: Path) -> dict: + """Refuse to start anything the resolved configuration would let reach the + host (SEC, 2026-10-08). The server checks the text it was sent; this checks + what compose resolved it to, and _up starts exactly that, so no second + reading of the text (or of a .env changed in between) can differ.""" + from ..composecheck import pin, problems + try: + resolved = pin(_resolved_config(workdir, project, compose_file), str(workdir)) + live_rw = _live_rw_binds(data_dir) + except Exception: # noqa: BLE001 — cannot check it, so do not run it + # compose's errors quote the line they could not parse, of whatever + # file that was: the detail stays in the host's log, not the result + log.warning("compose config failed for stack %s", project, exc_info=True) + raise ValueError("could not read the resolved compose configuration " + "(the agent's log on the host has compose's error)") from None + found = problems(resolved, str(workdir), live_rw) + if found: + raise ValueError("refusing to deploy " + project + ": " + "; ".join(found[:10])) + return resolved + + +def _up(workdir: Path, project: str, resolved: dict, data_dir: Path) -> str: + """``up`` on the checked configuration, kept in the agent's own data dir: + outside the stack folder (a container bound there could swap a file) and a + path no stack may bind. It stays, because compose records it in the + containers' config_files label and stack_update re-runs ``up`` from it.""" + import json + + resolved_dir = Path(data_dir) / "stacks" + if resolved_dir.is_symlink(): + raise ValueError(f"refusing to deploy: {resolved_dir} is a symlink") + resolved_dir.mkdir(mode=0o700, parents=True, exist_ok=True) + os.chmod(resolved_dir, 0o700) + path = resolved_dir / f"{project}.json" + _write_private(path, json.dumps(resolved)) + return ex._run([*ex._compose_cmd(), "-p", project, "--project-directory", str(workdir), + "-f", str(path), "up", "-d", "--remove-orphans"], + timeout=600, extra_env=ex._compose_env()) + + def _stack_deploy(params: dict, config: AgentConfig) -> str: from .. import client @@ -80,15 +345,23 @@ def _stack_deploy(params: dict, config: AgentConfig) -> str: if ticket and not _TICKET.match(ticket): raise ValueError("env_ticket must be a ticket id") compose_file = _compose_file(params) - # Redeemed first: a deploy that cannot have its secrets must not start - # half-configured containers. + source = _source_params(params) + # Redeemed first: a deploy that cannot have its secrets (or its source) + # must not start half-configured containers. env_text = client.fetch_stack_env(config, ticket) if ticket else None + files = _source_files(client.fetch_stack_source(config, source[0]), source[1]) if source else None + _safe_workdir(workdir) workdir.mkdir(parents=True, exist_ok=True) - (workdir / compose_file).write_text(compose) + if files is not None: + # The repo's folder first; the compose file and .env below are then + # written over it — the compose text is the one the server checked. + _place_source(workdir, files, Path(config.data_dir) / "stacks" / f"{project}.source.json") + _write_new(workdir / compose_file, compose, 0o644) if env_text is not None: _write_private(workdir / ".env", env_text) - output = _compose(workdir, project, "up", "-d", "--remove-orphans", compose_file=compose_file) + resolved = _check_resolved(workdir, project, compose_file, config.data_dir) + output = _up(workdir, project, resolved, config.data_dir) collector.request_docker_recheck() revision = params.get("revision") return ActionOutput(output or f"deployed {project}", @@ -96,7 +369,7 @@ def _stack_deploy(params: dict, config: AgentConfig) -> str: "revision": revision if isinstance(revision, int) else 0}) -def _stack_remove(params: dict, _config: AgentConfig) -> str: +def _stack_remove(params: dict, config: AgentConfig) -> str: project = _project(params) workdir = _workdir(params) delete = params.get("delete_files") @@ -107,6 +380,10 @@ def _stack_remove(params: dict, _config: AgentConfig) -> str: raise ValueError(f"refusing to delete {workdir}: only {STACKS_ROOT}/ is Vigil's") output = _compose(workdir, project, "down", timeout=300) if (workdir / COMPOSE_NAME).exists() \ else f"{project}: no compose file at {workdir} — nothing to take down" + # the configuration _up checked and started, kept for the containers' labels, + # and the list of source files a Git stack's deploys placed + for kept in (f"{project}.json", f"{project}.source.json"): + (Path(config.data_dir) / "stacks" / kept).unlink(missing_ok=True) deleted = False if delete: if workdir.exists(): @@ -116,16 +393,28 @@ def _stack_remove(params: dict, _config: AgentConfig) -> str: return ActionOutput(output, {"project": project, "files_deleted": deleted}) -def _hash_report(project: str, args: list[str]) -> dict: +def _hash_report(project: str, workdir: Path, compose: str, env: str) -> dict: """Which services would compose recreate? ``config --hash '*'`` against - the config-hash label on each running container. Matching = untouched.""" + the config-hash label on each running container. Matching = untouched. + + Run on private copies of the files _stack_read already read safely, not on + the labelled paths (a second, link-following read as root); and compose's + error text is never passed on — its parse errors quote the offending line, + of whatever file it was.""" + import tempfile + from .containers import _COMPOSE_PROJECT_LABEL - try: - out = ex._run([*ex._compose_cmd(), *args, "config", "--hash", "*"], timeout=120, - extra_env=ex._compose_env()) - except RuntimeError as exc: - return {"error": str(exc)[:300]} + with tempfile.TemporaryDirectory(prefix="vigil-adopt-") as tmp: + compose_copy, env_copy = Path(tmp) / "compose.yaml", Path(tmp) / "stack.env" + _write_private(compose_copy, compose) + _write_private(env_copy, env) + try: + out = ex._run([*ex._compose_cmd(), "-p", project, "--project-directory", str(workdir), + "--env-file", str(env_copy), "-f", str(compose_copy), + "config", "--hash", "*"], timeout=120, extra_env=ex._compose_env()) + except RuntimeError: + return {"error": "compose could not resolve this stack as it is on the host"} wanted = {} for line in out.splitlines(): parts = line.split() @@ -159,15 +448,20 @@ def _stack_read(params: dict, config: AgentConfig) -> str: compose_path = Path(files[0]) workdir = Path(args[args.index("--project-directory") + 1]) if "--project-directory" in args \ else compose_path.parent - env_path = workdir / ".env" + env_path = Path(args[args.index("--env-file") + 1]) if "--env-file" in args \ + else workdir / ".env" + compose = _read_plain(compose_path) + if compose is None: + raise ValueError(f"{compose_path} does not exist") payload = { "project": project, "compose_file": compose_path.name, "working_dir": str(workdir), - "compose": compose_path.read_text(), - "env": env_path.read_text() if env_path.exists() else "", - "hashes": _hash_report(project, args), + "compose": compose, + "env": _read_plain(env_path) or "", + "hashes": None, } + payload["hashes"] = _hash_report(project, workdir, compose, payload["env"]) client.post_stack_read(config, ticket, payload) report = payload["hashes"] return ActionOutput( diff --git a/agent/vigil_agent/client.py b/agent/vigil_agent/client.py index f23ab0e5..8dd5a772 100644 --- a/agent/vigil_agent/client.py +++ b/agent/vigil_agent/client.py @@ -4,6 +4,7 @@ """ import logging +import os import platform import socket @@ -72,6 +73,12 @@ def checkin( "allowlist": sorted(config.allowlist), "allow_reprovision": bool(config.allow_reprovision), } + # A managed agent left on the monitor-mode unit runs unprivileged and fails + # every task; the server marks such a host as refusing them (QA-08). + try: + payload["runs_as_root"] = os.geteuid() == 0 + except AttributeError: # Windows + pass # The ingest distinguishes an absent key (agent too old to report it — # stored value left alone) from an explicit False, so the key is only # sent when the probe produced a value. @@ -187,6 +194,26 @@ def fetch_stack_env(config: AgentConfig, ticket: str) -> str: return str(resp.json().get("env") or "") +#: A Git stack's packed source folder (the server caps it at 20 MB). +MAX_STACK_SOURCE_BYTES = 20 * 1024 * 1024 + + +def fetch_stack_source(config: AgentConfig, ticket: str) -> bytes: + """Redeem a deploy's one-time source ticket (2026.14.1) for the packed + folder of a Git stack. The caller checks it against the signed sha256.""" + url = f"{config.server_url}/api/v1/agent/stack-source/{ticket}/" + with requests.get(url, headers=_headers(config), timeout=_TIMEOUT, stream=True) as resp: + if resp.status_code != 200: + raise RuntimeError(f"the server would not hand over this deploy's source " + f"({resp.status_code}) — the ticket is used or expired; deploy again") + body = bytearray() + for chunk in resp.iter_content(64 * 1024): + body += chunk + if len(body) > MAX_STACK_SOURCE_BYTES: + raise RuntimeError("the stack's source is larger than an agent accepts") + return bytes(body) + + def post_stack_read(config: AgentConfig, ticket: str, payload: dict) -> None: """Hand an adopted stack's files to the server (M11) — over the agent's own connection, never in the task result, because .env holds secrets.""" diff --git a/agent/vigil_agent/composecheck.py b/agent/vigil_agent/composecheck.py new file mode 100644 index 00000000..e2bba2a8 --- /dev/null +++ b/agent/vigil_agent/composecheck.py @@ -0,0 +1,424 @@ +"""Check what compose will actually run, just before it runs it. + +The server validates the compose text it is sent (apps/stacks/validation.py), +but compose re-reads that text: it substitutes variables from the .env and +the environment, merges include/extends, normalises paths and booleans. Any +difference between the two readings is a way past the server's check. So the +agent asks compose for its resolved configuration (``config --format json``), +checks that, and then starts *that file* — the checked bytes are the ones +``up`` reads (stacks._stack_deploy). + +The rules accept every shape compose allows, not just the one docker prints: +podman-compose's ``config`` hands back the file much as it was written (short +volume strings, list sysctls, "true" strings). A shape not understood here is +refused, and so is any service or top-level key not on the allowlist. +""" +from __future__ import annotations + +import os +import posixpath +import re + +FORBIDDEN_BINDS = ("/run", "/var/run", "/etc", "/root", "/boot", "/proc", "/sys", "/dev", "/usr", + "/bin", "/sbin", "/lib", "/lib64", "/var/lib/docker", "/var/lib/containers", + "/var/lib/containerd", "/var/lib/vigil-agent", "/var/spool/cron", "/opt/vigil") +FORBIDDEN_CAPS = {"ALL", "SYS_ADMIN", "SYS_MODULE", "SYS_PTRACE", "SYS_RAWIO", "SYS_BOOT", + "DAC_READ_SEARCH", "BPF", "PERFMON", "MAC_ADMIN", "MAC_OVERRIDE"} +DEVICE_RE = re.compile(r"/dev/dri(/(card|renderD)\d+)?|/dev/nvidia(\d+|ctl|-uvm|-uvm-tools|-modeset)" + r"|/dev/net/tun|/dev/fuse|/dev/kfd") + +#: The same allowlist as the server's validation.py, less what it refuses. +#: A key compose adds later is refused here until someone has looked at it. +SERVICE_KEYS = { + "image", "command", "entrypoint", "environment", "ports", "expose", "restart", + "depends_on", "networks", "healthcheck", "labels", "logging", "container_name", "hostname", + "domainname", "user", "working_dir", "stop_signal", "stop_grace_period", "deploy", "cap_drop", + "dns", "dns_search", "dns_opt", "extra_hosts", "tmpfs", "ulimits", "shm_size", "mem_limit", + "memswap_limit", "mem_reservation", "cpus", "cpu_shares", "cpuset", "cpu_count", "cpu_percent", + "pids_limit", "read_only", "init", "tty", "stdin_open", "platform", "pull_policy", "profiles", + "group_add", "oom_score_adj", "oom_kill_disable", "mac_address", "links", "attach", "scale", "gpus", + # checked below + "build", "env_file", "volumes", "volumes_from", "devices", "cap_add", "security_opt", + "privileged", "pid", "ipc", "userns_mode", "cgroup", "uts", "network_mode", "sysctls", + "secrets", "configs", +} +TOP_LEVEL_KEYS = {"version", "name", "services", "networks", "volumes", "secrets", "configs"} +_TRUTHY = {"true", "yes", "y", "on", "1"} +_REMOTE_CONTEXT = ("http://", "https://", "git://", "git@", "ssh://") + + +def _norm(path: str) -> str: + return "/" + posixpath.normpath(str(path)).lstrip("/") + + +def _forbidden(path: str) -> bool: + path = _norm(path) + if path == "/": + return True + return any(path == p or path.startswith(p + "/") or p.startswith(path + "/") for p in FORBIDDEN_BINDS) + + +def bind_forbidden(source: str) -> bool: + """Is, is inside, or contains a protected path — as written, and once + symlinks on the host are followed (a container bound to its own stack + folder can plant ``data -> /`` there before the next deploy).""" + return _forbidden(source) or _forbidden(os.path.realpath(_norm(source))) + + +def _inside(path: str, root: str) -> bool: + root = _norm(root) + return all(p == root or p.startswith(root + "/") + for p in (_norm(path), os.path.realpath(_norm(path)))) + + +def _flag(value) -> bool: + return value is True or str(value).strip().lower() in _TRUTHY + + +def _items(value) -> list: + """A list as given, a lone string as a one-item list, nothing as [].""" + if value is None: + return [] + if isinstance(value, (list, tuple)): + return list(value) + return [value] + + +def _has_var(value) -> bool: + # In docker's output a literal $ is escaped as $$; podman-compose may hand + # back an unsubstituted one. Either way the file is read again by `up`, so + # a $ in a field that decides access could still change after this check. + return "$" in str(value) + + +def _abs(path: str, workdir: str) -> str: + path = str(path) + # "~" is the agent's home — never inside a stack, so leave it to be refused + return path if path.startswith(("/", "~")) else posixpath.normpath(posixpath.join(workdir, path)) + + +def _bind_problem(source: str, workdir: str) -> str | None: + if source.startswith("~"): + return f"bind of a home directory ({source})" + path = _abs(source, workdir) + link = _symlink_in(path, workdir) + if link: + return f"bind of {source}: {link} is a symlink" + loose = unsafe_dir(path) + if loose: + return (f"bind of {source}: {loose} can be changed by a user other than root " + f"(keep stack data in the stack folder or a root-owned path such as /srv)") + if _inside(path, workdir): # the stack's own folder, symlinks followed + return None + return f"bind of {source}" if bind_forbidden(path) else None + + +def unsafe_dir(path: str) -> str | None: + """The first folder above *path* (as written, and once links are + followed) that someone other than root could change: not root-owned, or + writable by group/others (sticky /tmp too). Whoever can write it can swap + a component of the path for a symlink after any check here, and the + engine — or the agent writing as root — would follow it.""" + for candidate in {_norm(path), os.path.realpath(_norm(path))}: + current = "/" + for part in candidate.strip("/").split("/")[:-1]: + current = posixpath.join(current, part) + try: + st = os.stat(current) + except FileNotFoundError: + break # created later by root + if st.st_uid != 0 or st.st_mode & 0o022: + return current + return None + + +def _symlink_in(path: str, workdir: str) -> str | None: + """The first symlink on the way from the stack folder down to *path*. A + container that can write the stack folder can plant one; the engine follows + it when it mounts, after any check here — so a link already there is + refused, and _covered() refuses a bind a running container could re-link.""" + path, root = _norm(path), _norm(workdir) + if not path.startswith(root + "/"): + return None + current = root + for part in path[len(root) + 1:].split("/"): + current = posixpath.join(current, part) + if os.path.islink(current): + return current + return None + + +def _volume_problem(vol, workdir: str) -> str | None: + if isinstance(vol, str): + parts = vol.split(":") + if len(parts) == 1: + return None # anonymous volume: "/data" + source = parts[0] + if _has_var(source): + return f"volume source {source} has a variable" + if source.startswith(("/", ".", "~")): + return _bind_problem(source, workdir) + return None # a named volume + if not isinstance(vol, dict): + return f"volume {vol!r} is not understood" + kind = vol.get("type") or "volume" + source = str(vol.get("source") or "") + if _has_var(source): + return f"volume source {source} has a variable" + if kind == "bind" or source.startswith(("/", ".", "~")): + if not source: + return "bind without a source" + return _bind_problem(source, workdir) + if kind not in ("volume", "tmpfs", "image"): + return f"volume type {kind}" + return None + + +def _service(name: str, svc, names: set, workdir: str) -> list[str]: + where = f"service {name!r}" + if not isinstance(svc, dict): + return [f"{where} is not a mapping"] + out: list[str] = [] + for key, value in svc.items(): + if key not in SERVICE_KEYS and not str(key).startswith("x-") and value not in (None, "", [], {}): + out.append(f"{where}: {key} is not allowed") + if "privileged" in svc and (_has_var(svc["privileged"]) or _flag(svc["privileged"])): + out.append(f"{where}: privileged") + for key in ("pid", "ipc", "userns_mode", "cgroup", "uts", "network_mode"): + value = str(svc.get(key) or "").strip().lower() + if _has_var(value) or value.startswith("container:") or (value == "host" and key != "network_mode"): + out.append(f"{where}: {key}: {value}") + if key == "network_mode" and value.startswith("service:") and value.split(":", 1)[1] not in names: + out.append(f"{where}: network_mode {value}") + for cap in _items(svc.get("cap_add")): + if _has_var(cap) or str(cap).strip().upper().removeprefix("CAP_") in FORBIDDEN_CAPS: + out.append(f"{where}: cap_add {cap}") + for opt in _items(svc.get("security_opt")): + low = str(opt).strip().lower().replace("=", ":") + allowed = (low in ("no-new-privileges", "no-new-privileges:true") + or (low.startswith("apparmor:") and "unconfined" not in low) + or low.startswith("label:level:")) + if _has_var(opt) or not allowed: + out.append(f"{where}: security_opt {opt}") + for device in _items(svc.get("devices")): + source = str(device.get("source") or "") if isinstance(device, dict) else str(device).split(":", 1)[0] + if _has_var(source) or ".." in source.split("/") or not DEVICE_RE.fullmatch(_norm(source)) \ + or not DEVICE_RE.fullmatch(os.path.realpath(_norm(source))): + out.append(f"{where}: device {source}") + sysctls = svc.get("sysctls") or {} + keys = sysctls.keys() if isinstance(sysctls, dict) else [str(s).split("=", 1)[0] for s in _items(sysctls)] + for key in keys: + if _has_var(key) or not str(key).strip().startswith("net."): + out.append(f"{where}: sysctl {key}") + for ref in _items(svc.get("volumes_from")): + target = str(ref).removeprefix("service:").split(":", 1)[0] + if str(ref).startswith("container:") or target not in names: + out.append(f"{where}: volumes_from {ref}") + for vol in _items(svc.get("volumes")): + problem = _volume_problem(vol, workdir) + if problem: + out.append(f"{where}: {problem}") + for env_file in _items(svc.get("env_file")): + path = env_file.get("path") if isinstance(env_file, dict) else env_file + if path and not _inside(_abs(path, workdir), workdir): + out.append(f"{where}: env_file {path} is outside the stack") + out += _build(where, svc.get("build"), workdir) + return out + + +def _local_context(value) -> bool: + """A path on this host, not a remote repository, an image or another service.""" + return not str(value).startswith((*_REMOTE_CONTEXT, "docker-image://", "service:")) + + +def _build(where: str, build, workdir: str) -> list[str]: + if build is None: + return [] + if isinstance(build, str): + build = {"context": build} + if not isinstance(build, dict): + return [f"{where}: build is not understood"] + out = [] + context = build.get("context") or "." + extra = build.get("additional_contexts") or {} + if isinstance(extra, list): # ["name=path", ...] + extra = dict(str(e).split("=", 1) if "=" in str(e) else (str(e), "/") for e in extra) + if not isinstance(extra, dict): + return [f"{where}: build.additional_contexts is not understood"] + paths = [("context", context), *((f"additional_contexts.{k}", v) for k, v in extra.items())] + for label, value in paths: + if _has_var(value): + out.append(f"{where}: build.{label} has a variable") + elif _local_context(value) and not _inside(_abs(value, workdir), workdir): + out.append(f"{where}: build.{label} {value} is outside the stack") + dockerfile = build.get("dockerfile") + if dockerfile and _local_context(context): + base = _abs(context, workdir) + if _has_var(dockerfile) or not _inside(_abs(dockerfile, base), workdir): + out.append(f"{where}: build.dockerfile {dockerfile} is outside the stack") + for key in ("ssh", "entitlements", "privileged"): + if build.get(key): + out.append(f"{where}: build.{key}") + if str(build.get("network") or "") not in ("", "default", "none"): + out.append(f"{where}: build.network {build['network']}") + return out + + +#: Fields whose strings only reach inside the container: a $ there may stay +#: ($$ in docker's output re-reads as a literal $). Anywhere else a $ is +#: refused — compose reads the checked file again, and could substitute it. +INERT_KEYS = {"environment", "command", "entrypoint", "healthcheck", "labels", "content"} + + +def _variables(node, where: str, out: list[str], depth: int = 0, seen=None) -> None: + # podman-compose's YAML can share nodes through anchors: walk each once + seen = set() if seen is None else seen + if isinstance(node, (dict, list)): + if id(node) in seen: + return + seen.add(id(node)) + if depth > 64: + out.append(f"{where}: nested too deeply") + elif isinstance(node, dict): + for key, value in node.items(): + if str(key).lower().startswith("x-podman"): + # inert to docker, but podman-compose acts on it (podman_args, pod_args, …) + out.append(f"{where}: {key} is not allowed") + elif _has_var(key): + out.append(f"{where}: key {key!r} has a variable") + elif key not in INERT_KEYS and not str(key).startswith("x-"): + _variables(value, f"{where}.{key}", out, depth + 1, seen) + elif isinstance(node, list): + for value in node: + _variables(value, where, out, depth + 1, seen) + elif _has_var(node): + out.append(f"{where}: {node!r} has a variable") + + +def _pin_volume(vol, workdir: str): + if isinstance(vol, str): + parts = vol.split(":") + if len(parts) > 1 and parts[0].startswith("."): + return ":".join([_abs(parts[0], workdir), *parts[1:]]) + return vol + if isinstance(vol, dict) and str(vol.get("source") or "").startswith("."): + return {**vol, "source": _abs(vol["source"], workdir)} + return vol + + +def pin(config: dict, workdir: str) -> dict: + """The configuration with every relative host path made absolute against + the stack folder. docker's output already is; podman-compose's may not be, + and ``up`` reads the checked file from the agent's data dir, where compose + would resolve "./" and "../" against *that* folder instead. What is + checked and what is started is this pinned copy.""" + import copy + + config = copy.deepcopy(config) + if not isinstance(config, dict): + return config + for svc in (config.get("services") or {}).values() if isinstance(config.get("services"), dict) else (): + if not isinstance(svc, dict): + continue + if isinstance(svc.get("volumes"), list): + svc["volumes"] = [_pin_volume(v, workdir) for v in svc["volumes"]] + if "env_file" in svc: + svc["env_file"] = [{**e, "path": _abs(e["path"], workdir)} if isinstance(e, dict) and e.get("path") + else _abs(e, workdir) if isinstance(e, str) else e + for e in _items(svc["env_file"])] + build = svc.get("build") + if isinstance(build, str): + build = svc["build"] = {"context": build} + if isinstance(build, dict): + context = build.get("context") or "." + if _local_context(context): + build["context"] = _abs(context, workdir) + extra = build.get("additional_contexts") + if isinstance(extra, dict): + build["additional_contexts"] = {k: _abs(v, workdir) if _local_context(v) else v + for k, v in extra.items()} + for section in ("secrets", "configs"): + for spec in (config.get(section) or {}).values() if isinstance(config.get(section), dict) else (): + if isinstance(spec, dict) and spec.get("file"): + spec["file"] = _abs(spec["file"], workdir) + return config + + +def rw_binds(config: dict, workdir: str) -> list[str]: + """Host paths this configuration mounts writable.""" + out = [] + for svc in (config.get("services") or {}).values(): + for vol in _items((svc or {}).get("volumes")) if isinstance(svc, dict) else (): + if isinstance(vol, str): + parts = vol.split(":") # source:target[:options] + options = parts[2].split(",") if len(parts) > 2 else [] + if len(parts) > 1 and parts[0].startswith(("/", ".")) and "ro" not in options: + out.append(_norm(_abs(parts[0], workdir))) + elif isinstance(vol, dict) and str(vol.get("source") or "").startswith(("/", ".")) \ + and not _flag(vol.get("read_only")): + out.append(_norm(_abs(vol["source"], workdir))) + return out + + +def _sources(config: dict, workdir: str) -> list[str]: + out = [] + for svc in (config.get("services") or {}).values(): + for vol in _items((svc or {}).get("volumes")) if isinstance(svc, dict) else (): + source = vol.split(":")[0] if isinstance(vol, str) and ":" in vol \ + else str(vol.get("source") or "") if isinstance(vol, dict) else "" + if source.startswith(("/", ".")): + out.append(_norm(_abs(source, workdir))) + return out + + +def _covered(config: dict, workdir: str, live_rw: tuple) -> list[str]: + """A bind inside a folder some Vigil stack's container can write: that + container can swap in a symlink between this check and the mount.""" + writable = set(rw_binds(config, workdir)) | {_norm(p) for p in live_rw} + out = [] + root = _norm(workdir) + for w in sorted(writable): + if root == w or root.startswith(w.rstrip("/") + "/"): + # the agent writes compose.yaml and .env here as root + out.append(f"{w} is mounted writable and holds the stack folder; bind a subfolder instead") + for source in sorted(set(_sources(config, workdir))): + for real in {source, os.path.realpath(source)}: + above = [w for w in writable if real.startswith(w.rstrip("/") + "/")] + if above: + out.append(f"bind of {source} is inside {above[0]}, which a container can write") + break + return out + + +def problems(config: dict, workdir: str, live_rw: tuple = ()) -> list[str]: + """Every reason not to start this (pinned) resolved configuration ([] = + fine). *live_rw*: host paths running Vigil-stack containers mount writable.""" + if not isinstance(config, dict): + return ["the configuration is not a mapping"] + out: list[str] = [] + _variables(config, "config", out) + out += _covered(config, workdir, live_rw) + for key, value in config.items(): + if key not in TOP_LEVEL_KEYS and not str(key).startswith("x-") and value not in (None, "", [], {}): + out.append(f"top-level {key} is not allowed") + services = config.get("services") or {} + if not isinstance(services, dict): + return out + ["services is not a mapping"] + names = set(services) + for name, svc in services.items(): + out += _service(name, svc, names, workdir) + for vname, vol in (config.get("volumes") or {}).items(): + opts = (vol or {}).get("driver_opts") if isinstance(vol, dict) else None + if not isinstance(opts, dict): + continue + device = str(opts.get("device") or "") + kind = str(opts.get("type") or "").lower() + if any(_has_var(v) for v in opts.values()) or "bind" in str(opts.get("o") or "").lower() \ + or (device and kind not in ("tmpfs", "nfs", "nfs4", "cifs")): + out.append(f"volume {vname!r}: driver_opts {opts}") + for section in ("secrets", "configs"): + for item, spec in (config.get(section) or {}).items(): + path = (spec or {}).get("file") if isinstance(spec, dict) else None + if path and (_has_var(path) or not _inside(_abs(path, workdir), workdir)): + out.append(f"{section[:-1]} {item!r}: {path} is outside the stack") + return out diff --git a/agent/vigil_agent/config.py b/agent/vigil_agent/config.py index 2814c86a..247a69fa 100644 --- a/agent/vigil_agent/config.py +++ b/agent/vigil_agent/config.py @@ -183,6 +183,11 @@ class AgentConfig: # tags take precedence: this list is used to seed/augment, never to # overwrite tags an operator has set in the console. tags: list[str] = field(default_factory=list) + # The server's Ed25519 public key, written into agent.yml by the installer + # (SEC-3). When set it is the only key the agent accepts: no + # trust-on-first-use, and a pin file in data_dir (which a monitor-mode + # service account owns) cannot override it. + server_public_key: str = "" # Process names sampled at every scrape whether or not they rank in the # top ten. Without this a quiet process is invisible: the collector only # ships the busiest processes, so a chart of one named service would be @@ -354,6 +359,16 @@ def load_config(path: Path | None = None) -> AgentConfig: data_dir = Path(raw.get("data_dir", "/var/lib/vigil-agent")) + server_public_key = str(raw.get("server_public_key") or "").strip() + if server_public_key: + import base64 + import binascii + try: + if len(base64.b64decode(server_public_key, validate=True)) != 32: + raise ValueError + except (ValueError, binascii.Error): + raise ValueError("server_public_key in config is not a base64 Ed25519 public key") from None + raw_tags = raw.get("tags") or [] if not isinstance(raw_tags, list): raise ValueError("tags must be a list of strings") @@ -390,6 +405,7 @@ def load_config(path: Path | None = None) -> AgentConfig: else _default_scripts_dir(), allowed_script_hashes=allowed_script_hashes, tags=raw_tags, + server_public_key=server_public_key, process_watch=raw_watch, gpu_extended=bool(raw.get("gpu_extended", False)), config_path=path, diff --git a/agent/vigil_agent/executor.py b/agent/vigil_agent/executor.py index c39819d9..00a9875c 100644 --- a/agent/vigil_agent/executor.py +++ b/agent/vigil_agent/executor.py @@ -625,17 +625,17 @@ def _execute_script(params: dict, config: AgentConfig, inputs: dict | None = Non if not script_path.is_file(): raise ValueError(f"Script not found: {script_name}") - if os.name == "posix": - # POSIX mode bits only. Python synthesises st_mode on Windows, so this - # check there is meaningless and its remedy — chmod — is not a command - # the operator has. Windows access is governed by the ACL on - # scripts_dir, which the installer restricts. - st = script_path.stat() - if st.st_mode & (stat.S_IWGRP | stat.S_IWOTH): - raise ValueError( - f"Script {script_name} is writable by group/others — refusing " - f"to execute. Run: chmod go-w {script_path}" - ) + # Whoever can change the script, or any directory above it up to + # scripts_dir, chooses what runs here as root/LocalSystem (SEC-2). + from .scripttrust import untrusted + problem = untrusted(script_path, scripts_dir) + if problem: + fix = (f"Run: chown -R root {scripts_dir} && chmod -R go-w {scripts_dir}" + if os.name == "posix" else + "Re-run the agent installer as an administrator; it locks the scripts folder.") + raise ValueError( + f"Refusing to run {script_name}: {problem}. {fix}" + ) return ActionOutput( _run([str(script_path)], extra_env=_input_env(inputs)), @@ -790,8 +790,8 @@ def _remove_firewall_rule(params: dict, _config: AgentConfig) -> str: if protocol not in ("tcp", "udp"): raise ValueError(f"Protocol must be tcp or udp, got {protocol!r}") action = str(params.get("action", "allow")).lower() - if action not in ("allow", "deny"): - raise ValueError(f"Action must be allow or deny, got {action!r}") + if action not in ("allow", "deny", "reject"): + raise ValueError(f"Action must be allow, deny or reject, got {action!r}") source = firewall.validate_source(params.get("source", "any")) # Optional: only WindowsBackend uses these, and it validates rule_id # itself (see firewall.validate_rule_name) before it ever reaches diff --git a/agent/vigil_agent/features.py b/agent/vigil_agent/features.py index 6dc1050a..9cbf4139 100644 --- a/agent/vigil_agent/features.py +++ b/agent/vigil_agent/features.py @@ -6,4 +6,8 @@ it is told to). """ -FEATURES = ("relevant", "branches", "boost") +# signed_v2 (SEC-4): the server signs when it sent a task and to which agent, +# and this agent refuses any task that is not signed that way. +# stack_source (2026.14.1): stack_deploy places a Git stack's source folder +# from a ticketed, sha256-checked archive. +FEATURES = ("relevant", "branches", "boost", "signed_v2", "stack_source") diff --git a/agent/vigil_agent/firewall.py b/agent/vigil_agent/firewall.py index 84339d99..525287c0 100644 --- a/agent/vigil_agent/firewall.py +++ b/agent/vigil_agent/firewall.py @@ -324,7 +324,7 @@ def remove_rule(self, port, protocol, action="allow", source="any", # backend. `name`/`rule_id` are accepted for contract uniformity but # unused -- firewalld rules are removed by their field-based match, # same as ufw. - if action == "deny": + if action in ("deny", "reject"): rule = self._reject_rich_rule(port, protocol, source) out = _run(["firewall-cmd", "--permanent", f"--remove-rich-rule={rule}"]) diff --git a/agent/vigil_agent/nonce_store.py b/agent/vigil_agent/nonce_store.py index 7addec9c..6cde37d1 100644 --- a/agent/vigil_agent/nonce_store.py +++ b/agent/vigil_agent/nonce_store.py @@ -1,7 +1,8 @@ """Persistent nonce tracking for replay protection. -Stores seen nonces in a flat file. Periodically prunes entries older than -the maximum TTL to prevent unbounded growth. +Stores each seen nonce with the time it may be forgotten: no earlier than the +task's own TTL has run out (SEC-4). A fixed one-hour memory let a task whose +TTL was longer be replayed once its nonce had been pruned. """ import logging @@ -10,9 +11,13 @@ logger = logging.getLogger("vigil.nonce") -_NONCE_FILENAME = "seen_nonces" -# Keep nonces for 1 hour — well beyond any reasonable task TTL (default 300s) -_MAX_AGE_SECONDS = 3600 +# A new file: entries are expiry times now, where "seen_nonces" held the time a +# nonce was recorded. Reading one as the other would forget nonces too early. +_NONCE_FILENAME = "seen_nonces.v2" +# Never forget a nonce sooner than this, whatever the task's TTL. +_MIN_KEEP_SECONDS = 3600 +# Margin past a task's TTL, for clock skew between server and agent. +_SKEW_SECONDS = 600 class NonceStore: @@ -43,13 +48,18 @@ def seen(self, nonce: str) -> bool: """Return True if this nonce was already used (replay attempt).""" return nonce in self._entries - def record(self, nonce: str) -> None: - """Mark a nonce as used.""" - self._entries[nonce] = time.time() + def record(self, nonce: str, ttl_seconds: int | float = 0) -> None: + """Mark a nonce as used, remembered until its task could no longer run.""" + try: + ttl = max(0.0, float(ttl_seconds)) + except (TypeError, ValueError): + ttl = 0.0 + keep = max(_MIN_KEEP_SECONDS, ttl + _SKEW_SECONDS) + self._entries[nonce] = time.time() + keep self._prune() self._save() def _prune(self) -> None: - """Remove nonces older than _MAX_AGE_SECONDS.""" - cutoff = time.time() - _MAX_AGE_SECONDS - self._entries = {n: ts for n, ts in self._entries.items() if ts > cutoff} + """Forget nonces whose keep-until time has passed.""" + now = time.time() + self._entries = {n: until for n, until in self._entries.items() if until > now} diff --git a/agent/vigil_agent/scripttrust.py b/agent/vigil_agent/scripttrust.py new file mode 100644 index 00000000..335b6a99 --- /dev/null +++ b/agent/vigil_agent/scripttrust.py @@ -0,0 +1,110 @@ +"""May the agent run this script file? (SEC-2) + +``execute_script`` runs a named script from the scripts directory as root or +LocalSystem. That is only safe while nobody but an administrator can change +the script or anything above it up to the scripts directory: whoever can +write any of them chooses what runs with full privilege. + +Linux checked only the file's own mode bits. Windows checked nothing, trusting +an installer ACL that was never applied, and C:\\ProgramData's default lets +any local user create the scripts folder and own it. Both now check the file +and every directory from it up to and including the scripts directory: + +* POSIX: owned by root, and not writable by group or others. +* Windows: owned by SYSTEM, Administrators or TrustedInstaller, and no allow + entry that applies to the object grants a write-type right to anyone else. + Read through PowerShell with SIDs, so it works on any display language. +""" +from __future__ import annotations + +import json +import os +import stat +import subprocess +from pathlib import Path + +#: SYSTEM, BUILTIN\Administrators, NT SERVICE\TrustedInstaller. +TRUSTED_SIDS = frozenset({ + "S-1-5-18", + "S-1-5-32-544", + "S-1-5-80-956008885-3418522649-1831038044-1853292631-2271478464", +}) + +#: WriteData, AppendData, WriteEA, WriteAttributes, Delete, WRITE_DAC, +#: WRITE_OWNER, and the generic write/all bits. +WRITE_MASK = (0x2 | 0x4 | 0x10 | 0x100 | 0x10000 | 0x40000 | 0x80000 + | 0x40000000 | 0x10000000) + +_PS = ["powershell.exe", "-NoProfile", "-NonInteractive", "-Command"] + +#: Owner SID and the allow entries that apply to the object itself (not +#: inherit-only ones), as JSON. {0} is the path, single quotes doubled. +_PS_ACL = ( + "$a = Get-Acl -LiteralPath '{0}'; " + "$sid = [System.Security.Principal.SecurityIdentifier]; " + "$rules = @($a.GetAccessRules($true, $true, $sid) | " + "Where-Object {{ $_.AccessControlType -eq 'Allow' -and " + "-not ($_.PropagationFlags -band [System.Security.AccessControl.PropagationFlags]::InheritOnly) }} | " + "ForEach-Object {{ @{{ sid = $_.IdentityReference.Value; rights = [int64]$_.FileSystemRights }} }}); " + "@{{ owner = $a.GetOwner($sid).Value; rules = $rules }} | ConvertTo-Json -Compress -Depth 4" +) + + +def chain(script: Path, root: Path) -> list[Path]: + """The script, then each directory above it, up to and including *root*.""" + out = [script] + cur = script.parent + while True: + out.append(cur) + if cur == root or cur.parent == cur: + break + cur = cur.parent + return out + + +def posix_problem(path: Path) -> str: + st = path.stat() + if st.st_uid != 0: + return f"{path} is not owned by root" + if st.st_mode & (stat.S_IWGRP | stat.S_IWOTH): + return f"{path} is writable by group or others" + return "" + + +def windows_acl(path: Path) -> dict: + """{'owner': sid, 'rules': [{'sid', 'rights'}]} for *path*.""" + quoted = str(path).replace("'", "''") + out = subprocess.run(_PS + [_PS_ACL.format(quoted)], capture_output=True, + text=True, timeout=30, check=True).stdout + data = json.loads(out) + rules = data.get("rules") or [] + if isinstance(rules, dict): # ConvertTo-Json unwraps a one-item array + rules = [rules] + return {"owner": str(data.get("owner") or ""), "rules": rules} + + +def windows_problem(path: Path, acl: dict) -> str: + if acl["owner"] not in TRUSTED_SIDS: + return f"{path} is owned by {acl['owner'] or 'an unknown account'}, not an administrator" + for rule in acl["rules"]: + sid = str(rule.get("sid") or "") + rights = int(rule.get("rights") or 0) + if sid not in TRUSTED_SIDS and rights & WRITE_MASK: + return f"{path} can be changed by {sid}" + return "" + + +def untrusted(script: Path, root: Path) -> str: + """'' when the script may run; otherwise why not. Fails closed: a check + that cannot be completed is a reason to refuse.""" + for path in chain(script, root): + try: + if os.name == "nt": + problem = windows_problem(path, windows_acl(path)) + else: + problem = posix_problem(path) + except Exception as exc: # noqa: BLE001 — any failure refuses + return f"could not check who can change {path}: {exc}" + if problem: + return problem + return "" diff --git a/agent/vigil_agent/verify.py b/agent/vigil_agent/verify.py index 0ac67fd7..c25c5086 100644 --- a/agent/vigil_agent/verify.py +++ b/agent/vigil_agent/verify.py @@ -27,13 +27,23 @@ def _pin_path(data_dir: Path) -> Path: return data_dir / _PIN_FILENAME -def pin_public_key(data_dir: Path, key_b64: str) -> VerifyKey: - """Pin the server's public key using TOFU. Returns the VerifyKey. +def pin_public_key(data_dir: Path, key_b64: str, configured: str = "") -> VerifyKey: + """Pin the server's public key. Returns the VerifyKey. - Raises KeyMismatchError if a different key was already pinned. + With *configured* (the key the installer wrote into agent.yml, SEC-3) that + key is the only one accepted: a different key from the server raises, and + the pin file is ignored. Without it, trust on first use: the first key seen + is pinned, and a different one later raises KeyMismatchError. """ - pin_file = _pin_path(data_dir) key_b64 = key_b64.strip() + if configured: + if key_b64 != configured.strip(): + raise KeyMismatchError( + "Server public key differs from the one this agent was installed with " + "(server_public_key in agent.yml). Re-run the installer if the server's " + "key was rotated on purpose.") + return VerifyKey(base64.b64decode(configured)) + pin_file = _pin_path(data_dir) if pin_file.exists(): stored = pin_file.read_text().strip() @@ -52,8 +62,11 @@ def pin_public_key(data_dir: Path, key_b64: str) -> VerifyKey: return VerifyKey(base64.b64decode(key_b64)) -def get_pinned_key(data_dir: Path) -> VerifyKey | None: - """Return the pinned VerifyKey, or None if no key is pinned yet.""" +def get_pinned_key(data_dir: Path, configured: str = "") -> VerifyKey | None: + """Return the pinned VerifyKey, or None if no key is pinned yet. The key + from agent.yml, when there is one, wins over the pin file.""" + if configured: + return VerifyKey(base64.b64decode(configured)) pin_file = _pin_path(data_dir) if not pin_file.exists(): return None @@ -61,7 +74,13 @@ def get_pinned_key(data_dir: Path) -> VerifyKey | None: return VerifyKey(base64.b64decode(stored)) -def verify_task_signature(task: dict, verify_key: VerifyKey) -> bool: +def agent_fingerprint(agent_token: str) -> str: + """Must match server/vigil/signing.py:agent_fingerprint.""" + import hashlib + return hashlib.sha256(agent_token.encode()).hexdigest() + + +def verify_task_signature(task: dict, verify_key: VerifyKey, agent_token: str | None = None) -> bool: """Verify the Ed25519 signature on a task payload. The canonical payload must match exactly what the server signs @@ -72,18 +91,25 @@ def verify_task_signature(task: dict, verify_key: VerifyKey) -> bool: logger.warning("Task %s has no signature — rejecting", task.get("id")) return False + fields = { + "id": task["id"], + "host_id": task.get("host_id", ""), + "action": task["action"], + "params": task.get("params", {}), + "nonce": task["nonce"], + "ttl_seconds": task.get("ttl_seconds", 300), + } + if agent_token is not None: + # This agent advertises signed_v2, so only a v2 signature counts: one + # covering when the task was sent and that it was sent to this agent. + # Accepting v1 here would let whoever relays a task strip both. + if task.get("sig_v") != 2 or not isinstance(task.get("dispatched_at"), str): + logger.warning("Task %s is not v2-signed — rejecting", task.get("id")) + return False + fields.update({"dispatched_at": task["dispatched_at"], + "agent": agent_fingerprint(agent_token), "v": 2}) # Reconstruct the canonical payload the server signs - canonical = json.dumps( - { - "id": task["id"], - "host_id": task.get("host_id", ""), - "action": task["action"], - "params": task.get("params", {}), - "nonce": task["nonce"], - "ttl_seconds": task.get("ttl_seconds", 300), - }, - sort_keys=True, - ).encode() + canonical = json.dumps(fields, sort_keys=True).encode() try: signature = base64.b64decode(signature_b64) diff --git a/docs/EDITIONS.md b/docs/EDITIONS.md index de545482..d90de92a 100644 --- a/docs/EDITIONS.md +++ b/docs/EDITIONS.md @@ -20,28 +20,28 @@ with `seats: 9999`). SSO/SAML is "available — talk to us," not a tier. | Feature | Free | Business | |---|:--:|:--:| -| Monitoring, alerting (smart defaults), unlimited agents/hosts/retention | ✅ | ✅ | -| Remote task execution + agent (agent is 100% OSS, no tier awareness) | ✅ | ✅ | -| Vuln scanning (Trivy / Nessus / Greenbone) | ✅ | ✅ | -| Vigil's own vulnerability matcher (OSV / KEV / EPSS, offline bundle), evidence and fix groups | ✅ | ✅ | -| Remediation by fix (deploy a fix to every affected host), AI mitigations when there is no fix | ✅ | ✅ | -| Detection tasks (severity / cves / boost), Organization · Community · Vendor tags, Anvil SARIF import | ✅ | ✅ | -| Containers & stacks (Docker / Podman): lifecycle, logs, rollback, single-host stack editor and .env, adopt | ✅ | ✅ | -| Audit trail of stack and .env events (viewer) | — | ✅ | -| **Baselines** (auto-dispatch on host approval) | ✅ | ✅ | -| **AI suggestions** — BYO endpoint, any OpenAI-compatible or Anthropic | ✅ | ✅ | -| **Status page** (public token URL, Powered-by-Vigil badge) | ✅ | ✅ | -| Jackil integration (alert → ticket) | ✅ | ✅ | -| **App & patch policies** (present / latest / pinned / absent, patching windows, deferral, approve each change) | ✅ | ✅ | -| Fleet view by update, fleet-wide approve / decline | ✅ | ✅ | -| Patch compliance numbers (% of hosts patched within SLA) | ✅ | ✅ | -| **Compliance report** — pass/fail per site, CSV export, branded printable report with "Licensed to" | — | ✅ | +| Monitoring, alerting (smart defaults), unlimited agents/hosts/retention | Yes | Yes | +| Remote task execution + agent (agent is 100% OSS, no tier awareness) | Yes | Yes | +| Vuln scanning (Trivy / Nessus / Greenbone) | Yes | Yes | +| Vigil's own vulnerability matcher (OSV / KEV / EPSS, offline bundle), evidence and fix groups | Yes | Yes | +| Remediation by fix (deploy a fix to every affected host), AI mitigations when there is no fix | Yes | Yes | +| Detection tasks (severity / cves / boost), Organization · Community · Vendor tags, Anvil SARIF import | Yes | Yes | +| Containers & stacks (Docker / Podman): lifecycle, logs, rollback, single-host stack editor and .env, adopt | Yes | Yes | +| Audit trail of stack and .env events (viewer) | — | Yes | +| **Baselines** (auto-dispatch on host approval) | Yes | Yes | +| **AI suggestions** — BYO endpoint, any OpenAI-compatible or Anthropic | Yes | Yes | +| **Status page** (public token URL, Powered-by-Vigil badge) | Yes | Yes | +| Jackil integration (alert → ticket) | Yes | Yes | +| **App & patch policies** (present / latest / pinned / absent, patching windows, deferral, approve each change) | Yes | Yes | +| Fleet view by update, fleet-wide approve / decline | Yes | Yes | +| Patch compliance numbers (% of hosts patched within SLA) | Yes | Yes | +| **Compliance report** — pass/fail per site, CSV export, branded printable report with "Licensed to" | — | Yes | | Seats | 1 admin + 1 viewer | **per-seat pricing** | | Roles | Admin, Viewer | + **Operator**, custom roles | | **Sites** (administrative boundaries; per-site scoping) | 1 (default) | **unlimited** | -| **Audit log** viewer + CSV export (recording is always on) | — | ✅ | -| **Status page branding** (logo, no badge, per-site client pages) | — | ✅ | -| **Branding** (reports, portal) | — | ✅ | +| **Audit log** viewer + CSV export (recording is always on) | — | Yes | +| **Status page branding** (logo, no badge, per-site client pages) | — | Yes | +| **Branding** (reports, portal) | — | Yes | | SSO/SAML | — | talk to us | **Seats are the only meter.** Sites are unlimited on Business because seats diff --git a/server/Dockerfile b/server/Dockerfile index 140a8219..90894d19 100644 --- a/server/Dockerfile +++ b/server/Dockerfile @@ -31,8 +31,10 @@ ENV PYTHONDONTWRITEBYTECODE=1 \ WORKDIR /app +# git + openssh-client (+ util-linux for prlimit): stacks from Git (2026.14.1) are fetched by the server, +# never by a host, so a repository credential stays in Vigil. RUN apt-get update && \ - apt-get install -y --no-install-recommends libpq-dev gosu && \ + apt-get install -y --no-install-recommends libpq-dev gosu git openssh-client util-linux && \ rm -rf /var/lib/apt/lists/* # Gunicorn and every request it serves ran as UID 0, so a code-execution bug in diff --git a/server/apps/accounts/apps.py b/server/apps/accounts/apps.py index 7d075303..47352bf5 100644 --- a/server/apps/accounts/apps.py +++ b/server/apps/accounts/apps.py @@ -5,3 +5,8 @@ class AccountsConfig(AppConfig): default_auto_field = "django.db.models.BigAutoField" name = "apps.accounts" label = "accounts" + + def ready(self): + from . import ( + session_timeout, # noqa: F401 — registers the user_logged_in receiver + ) diff --git a/server/apps/accounts/migrations/0006_totp_attempt.py b/server/apps/accounts/migrations/0006_totp_attempt.py new file mode 100644 index 00000000..1d2b6389 --- /dev/null +++ b/server/apps/accounts/migrations/0006_totp_attempt.py @@ -0,0 +1,24 @@ +# Generated by Django 5.2.17 on 2026-10-08 18:40 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('accounts', '0005_userprofile_role'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='TotpAttempt', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created_at', models.DateTimeField(auto_now_add=True, db_index=True)), + ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='+', to=settings.AUTH_USER_MODEL)), + ], + ), + ] diff --git a/server/apps/accounts/models.py b/server/apps/accounts/models.py index ec0237cb..5331029f 100644 --- a/server/apps/accounts/models.py +++ b/server/apps/accounts/models.py @@ -51,6 +51,19 @@ def __str__(self) -> str: return f"profile<{self.user_id}>" +class TotpAttempt(models.Model): + """One TOTP code tried for a user, kept briefly to rate-limit guessing. + + A table of its own, keyed by the user: sharing LoginAttempt let a failed + *login* typed with the right username land in the same count, so anyone + could lock a user out of TOTP-gated actions (architect review, 2026-10-08). + """ + + user = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.CASCADE, + related_name="+") + created_at = models.DateTimeField(auto_now_add=True, db_index=True) + + class LoginAttempt(models.Model): """A failed console login, kept briefly to rate-limit brute forcing. diff --git a/server/apps/accounts/session_timeout.py b/server/apps/accounts/session_timeout.py new file mode 100644 index 00000000..73944c00 --- /dev/null +++ b/server/apps/accounts/session_timeout.py @@ -0,0 +1,112 @@ +"""Sign-in sessions end after a stretch of idleness, and after a hard cap (QA-14). + +Only real use counts as activity: a page navigation, a change (any non-GET), +or the browser's heartbeat, which vigil-session.js sends when someone actually +touches the page. Background polling is checked but never refreshes the clock — +otherwise an open Monitor tab would keep a session alive forever. +""" + +import time +from urllib.parse import urlencode + +from django.contrib.auth import logout +from django.contrib.auth.signals import user_logged_in +from django.dispatch import receiver +from django.http import JsonResponse +from django.shortcuts import redirect +from django.urls import reverse + +LOGIN_AT = "vigil_login_at" +LAST_ACTIVE = "vigil_last_active" +HEARTBEAT_PATH = "/api/v1/accounts/session/activity/" +_SAFE = ("GET", "HEAD", "OPTIONS") +_ASSET_PREFIXES = ("/static/", "/media/") + + +def limits() -> tuple[int, int]: + """(idle seconds, max seconds) — the effective instance settings.""" + from apps.instance.config import setting + + idle = int(setting("VIGIL_SESSION_IDLE_MINUTES") or 15) + cap = int(setting("VIGIL_SESSION_MAX_HOURS") or 12) + return max(idle, 5) * 60, max(cap, 1) * 3600 + + +def is_activity(request) -> bool: + path = request.path + if path == HEARTBEAT_PATH: + return True + if request.method not in _SAFE: + return True + return not path.startswith("/api/") and not path.startswith(_ASSET_PREFIXES) + + +def remaining(session, now=None) -> dict: + """Seconds left before each limit ends the session (never negative).""" + now = now if now is not None else time.time() + idle, cap = limits() + last = session.get(LAST_ACTIVE, now) + start = session.get(LOGIN_AT, now) + return { + "idle_seconds_left": max(0, int(last + idle - now)), + "max_seconds_left": max(0, int(start + cap - now)), + "idle_seconds": idle, + "max_seconds": cap, + } + + +@receiver(user_logged_in) +def _stamp_login(sender, request, user, **kwargs): + now = time.time() + request.session[LOGIN_AT] = now + request.session[LAST_ACTIVE] = now + + +class SessionTimeoutMiddleware: + def __init__(self, get_response): + self.get_response = get_response + + def __call__(self, request): + user = getattr(request, "user", None) + if ( + user is not None + and user.is_authenticated + and hasattr(request, "session") + and not request.path.startswith(_ASSET_PREFIXES) + ): + now = time.time() + session = request.session + # A session from before this feature has no stamps: start its clocks now + # rather than signing everyone out on upgrade. + session.setdefault(LOGIN_AT, now) + session.setdefault(LAST_ACTIVE, now) + left = remaining(session, now) + reason = ( + "max" + if left["max_seconds_left"] <= 0 + else "idle" + if left["idle_seconds_left"] <= 0 + else "" + ) + if reason: + logout(request) + return self._signed_out(request, reason) + if is_activity(request): + session[LAST_ACTIVE] = now + return self.get_response(request) + + @staticmethod + def _signed_out(request, reason): + message = ( + "Signed out after a period of inactivity." + if reason == "idle" + else "Signed out — sign-ins last at most a set number of hours." + ) + if request.path.startswith("/api/"): + return JsonResponse( + {"detail": message, "reason": "session_" + reason}, status=401 + ) + query = {"expired": reason} + if request.method == "GET": + query["next"] = request.get_full_path() + return redirect(f"{reverse('login')}?{urlencode(query)}") diff --git a/server/apps/accounts/test_session_browser.py b/server/apps/accounts/test_session_browser.py new file mode 100644 index 00000000..b5c56a47 --- /dev/null +++ b/server/apps/accounts/test_session_browser.py @@ -0,0 +1,94 @@ +"""The browser half of the sign-in timeout (QA-14b) is wired the way it must be. + +There is no JS runtime in the suite, so these read the shipped files as text. +Each check defends one behaviour the feature depends on: the heartbeat rides +real use and never a timer, a tab re-asks the server before it frightens anyone +or leaves, and nothing reaches for an inline handler the CSP would refuse. +""" + +import re +from pathlib import Path + +from django.test import SimpleTestCase + +REPO = Path(__file__).resolve().parents[3] +TEMPLATE = REPO / "server" / "templates" / "base.html" +JS = REPO / "server" / "static" / "js" +SESSION = JS / "vigil-session.js" +UTILS = JS / "vigil-utils.js" + +#: Every way the file may start a repeating timer. A heartbeat on a timer would +#: keep a session alive with nobody at the keyboard, so only the tick may. +_TIMERS = re.compile(r"\b(?:setInterval|pollingInterval)\s*\(([^,)]+)") + + +def _body(src: str, name: str) -> str: + """A function's text, from its header to the next top-level definition. + + Exact enough for these checks because every function in the file is + top-level and in source order; brace-matching would only add ways to be + wrong about a file this small. + """ + start = src.index(f"function {name}(") + rest = src[start + 1:] + nxt = re.search(r"^(?:async )?function |^document\.addEventListener", rest, re.MULTILINE) + return rest[:nxt.start()] if nxt else rest + + +class SessionScriptLoadedTests(SimpleTestCase): + def test_script_is_loaded_after_modal(self): + html = TEMPLATE.read_text() + self.assertIn("js/vigil-session.js", html) + self.assertGreater( + html.index("js/vigil-session.js"), html.index("js/vigil-modal.js"), + "the timeout script needs mountModal, so it loads after the modal") + + +class HeartbeatTests(SimpleTestCase): + def test_heartbeat_only_on_real_use(self): + src = SESSION.read_text() + for event in ("pointerdown", "keydown", "wheel", "touchstart"): + self.assertIn(f"addEventListener('{event}', _sessionOnUse", src) + self.assertIn("/api/v1/accounts/session/", src) + self.assertIn("'activity/'", src) + self.assertEqual( + set(_TIMERS.findall(src)), {"_sessionTick"}, + "only the one-second tick may run on a timer — a heartbeat on a " + "timer would keep a session alive with nobody at the keyboard") + + def test_warning_window_is_two_minutes(self): + src = SESSION.read_text() + self.assertIn("SESSION_WARN_SECONDS = 120", src) + self.assertIn("HEARTBEAT_MS = 60 * 1000", src) + + +class RecheckTests(SimpleTestCase): + def test_rechecks_before_warning_and_leaving(self): + tick = _body(SESSION.read_text(), "_sessionTick") + self.assertGreaterEqual( + tick.count("_sessionFetch('GET')"), 2, + "a tab asks the server before warning and before leaving: another " + "tab's heartbeat extends this one") + + +class LeaveTests(SimpleTestCase): + def test_leave_keeps_the_place(self): + leave = _body(SESSION.read_text(), "_sessionLeave") + self.assertIn("/login/?expired=", leave) + self.assertIn("encodeURIComponent(location.pathname", leave) + + +class CspTests(SimpleTestCase): + def test_no_inline_handlers_or_html_injection(self): + src = SESSION.read_text() + for handler in ("onclick=", "onchange="): + self.assertNotIn(handler, src) + # The countdown is re-written every second, so it must never be markup. + warn = _body(src, "_sessionSetMessage") + self.assertIn("msg.textContent = _sessionMessage(", warn) + self.assertNotIn("innerHTML", warn) + + def test_apijson_redirects_on_session_401(self): + utils = UTILS.read_text() + self.assertIn("body.reason.startsWith('session_')", utils) + self.assertIn("_sessionLeave(", utils) diff --git a/server/apps/accounts/test_session_timeout.py b/server/apps/accounts/test_session_timeout.py new file mode 100644 index 00000000..77c198f6 --- /dev/null +++ b/server/apps/accounts/test_session_timeout.py @@ -0,0 +1,158 @@ +"""Sign-in sessions end on idleness and on a hard cap (QA-14a). + +The tests move time with a patched ``time.time`` rather than waiting for it. +The distinction the suite defends is the one the feature turns on: a +background ``GET /api/...`` poll is *checked* against the limits but must not +refresh the idle clock, or an open Monitor tab would never time out. +""" + +from unittest import mock + +from django.contrib.auth import get_user_model +from django.test import TestCase + +from apps.hosts.models import Host +from apps.instance import config + +from . import session_timeout + +T0 = 1_700_000_000.0 +MIN = 60 +API = "/api/v1/accounts/session/" +HEARTBEAT = "/api/v1/accounts/session/activity/" + + +class _ClockMixin: + def login(self): + """Sign in with the clock frozen at T0, and leave it frozen there. + + ``at()`` moves time only forward, so a test that never calls it still + runs at T0 rather than at the wall clock. + """ + user = get_user_model().objects.create_user("alice", password="pw") + self._patcher = mock.patch( + "apps.accounts.session_timeout.time.time", return_value=T0 + ) + self._patcher.start() + self.addCleanup(self._patcher.stop) + self.client.force_login(user) + return user + + def at(self, seconds): + """Freeze the middleware's clock at T0 + *seconds*.""" + self._patcher = mock.patch( + "apps.accounts.session_timeout.time.time", return_value=T0 + seconds + ) + self._patcher.start() + self.addCleanup(self._patcher.stop) + + +class LoginStampTests(_ClockMixin, TestCase): + def test_login_stamps_both_clocks(self): + self.login() + session = self.client.session + self.assertEqual(session[session_timeout.LOGIN_AT], T0) + self.assertEqual(session[session_timeout.LAST_ACTIVE], T0) + + +class IdleTimeoutTests(_ClockMixin, TestCase): + def test_background_poll_does_not_extend(self): + self.login() + self.at(10 * MIN) + self.assertEqual(self.client.get(API).status_code, 200) + self.at(16 * MIN) + resp = self.client.get(API) + self.assertEqual(resp.status_code, 401) + self.assertEqual(resp.json()["reason"], "session_idle") + + def test_heartbeat_extends(self): + self.login() + self.at(10 * MIN) + resp = self.client.post(HEARTBEAT) + self.assertEqual(resp.status_code, 200, getattr(resp, "data", None)) + self.assertEqual(resp.json()["idle_seconds_left"], 900) + self.at(20 * MIN) + self.assertEqual(self.client.get(API).status_code, 200) + + def test_page_load_extends(self): + self.login() + self.at(10 * MIN) + resp = self.client.get("/") + self.assertIn(resp.status_code, (200, 302)) + self.assertNotIn("/login/", resp.get("Location", "")) + self.at(20 * MIN) + self.assertEqual(self.client.get(API).status_code, 200) + + def test_idle_page_redirects_to_login(self): + self.login() + self.at(16 * MIN) + resp = self.client.get("/") + self.assertEqual(resp.status_code, 302) + self.assertEqual( + resp["Location"], + "/login/?expired=idle&next=%2F", + "the browser side keys off these query params (QA-14b)", + ) + self.at(16 * MIN + 1) + self.assertIn(self.client.get(API).status_code, (401, 403)) + + +class HardCapTests(_ClockMixin, TestCase): + def test_hard_cap(self): + self.login() + for minutes in range(10, 12 * 60, 10): + self.at(minutes * MIN) + self.assertEqual(self.client.post(HEARTBEAT).status_code, 200) + self.at(12 * 60 * MIN + MIN) + resp = self.client.post(HEARTBEAT) + self.assertEqual(resp.status_code, 401) + self.assertEqual(resp.json()["reason"], "session_max") + + +class SettingsChangeTests(_ClockMixin, TestCase): + def test_settings_change_applies(self): + config.write("VIGIL_SESSION_IDLE_MINUTES", "30") + config.invalidate() + self.login() + self.at(20 * MIN) + self.assertEqual(self.client.get(API).status_code, 200) + + +class ValidateRangeTests(TestCase): + def test_validate_ranges(self): + self.assertFalse(config.validate("VIGIL_SESSION_IDLE_MINUTES", "4")[0]) + self.assertTrue(config.validate("VIGIL_SESSION_IDLE_MINUTES", "5")[0]) + self.assertFalse(config.validate("VIGIL_SESSION_IDLE_MINUTES", "721")[0]) + self.assertFalse(config.validate("VIGIL_SESSION_MAX_HOURS", "0")[0]) + self.assertTrue(config.validate("VIGIL_SESSION_MAX_HOURS", "168")[0]) + + +class AgentsUnaffectedTests(TestCase): + def test_agents_unaffected(self): + """Agents authenticate by bearer token and carry no session, so the + middleware must leave them alone however long a host has been quiet.""" + host = Host.objects.create( + hostname="legacy-box", + agent_token="REPLACE_WITH_TOKEN", + status=Host.Status.ONLINE, + ) + with mock.patch( + "apps.accounts.session_timeout.time.time", return_value=T0 + 30 * 24 * 3600 + ): + resp = self.client.post( + "/api/v1/checkin", + {}, + format="json", + HTTP_AUTHORIZATION=f"Bearer {host.raw_agent_token}", + ) + self.assertEqual(resp.status_code, 200, getattr(resp, "data", None)) + self.assertNotIn("session_", resp.content.decode("utf-8", "ignore")) + + +class LoginPageTests(TestCase): + def test_login_page_says_why(self): + # Unauthenticated: an authenticated client is bounced off /login/ to + # the dashboard by login_view itself. + get_user_model().objects.create_user("alice", password="pw") + body = self.client.get("/login/?expired=idle").content.decode() + self.assertIn("period of inactivity", body) diff --git a/server/apps/accounts/test_totp_rate_limit.py b/server/apps/accounts/test_totp_rate_limit.py new file mode 100644 index 00000000..7e40eb4a --- /dev/null +++ b/server/apps/accounts/test_totp_rate_limit.py @@ -0,0 +1,84 @@ +"""TOTP-gated actions stop accepting codes after repeated wrong ones (architect +review, 2026-10-08). The gate exists for a session that someone holds without +the device; without a limit, a six-digit code falls to guessing.""" +from django.contrib.auth import get_user_model +from django.test import TestCase +from django.utils.timezone import now + +from apps.accounts.models import UserProfile +from apps.accounts.totp import MAX_TOTP_FAILURES, consume_totp, generate_secret, generate_totp + + +class TotpRateLimitTests(TestCase): + def setUp(self): + self.user = get_user_model().objects.create_user("u", password="pw") + profile = UserProfile.objects.create(user=self.user) + self.secret = generate_secret() + profile.totp_secret = self.secret + profile.totp_confirmed_at = now() + profile.save() + + def _wrong(self): + good = generate_totp(self.secret) + return "000000" if good != "000000" else "111111" + + def test_after_the_limit_even_the_right_code_is_refused(self): + for _ in range(MAX_TOTP_FAILURES): + self.assertFalse(consume_totp(self.user, self._wrong())[0]) + ok, err = consume_totp(self.user, generate_totp(self.secret)) + self.assertFalse(ok) + self.assertIn("Too many wrong codes", err) + + def test_a_right_code_before_the_limit_clears_the_count(self): + for _ in range(MAX_TOTP_FAILURES - 1): + consume_totp(self.user, self._wrong()) + self.assertTrue(consume_totp(self.user, generate_totp(self.secret))[0]) + for _ in range(MAX_TOTP_FAILURES - 1): + consume_totp(self.user, self._wrong()) + self.assertNotIn("Too many", consume_totp(self.user, self._wrong())[1]) + + def test_failed_logins_cannot_lock_someone_out(self): + # Failed sign-ins with a crafted username used to share the count. + from apps.accounts.models import LoginAttempt + for _ in range(20): + LoginAttempt.objects.create(username=f"totp:{self.user.pk}", ip="203.0.113.9") + self.assertTrue(consume_totp(self.user, generate_totp(self.secret))[0]) + + def test_an_attempt_is_recorded_before_the_code_is_checked(self): + # Record-then-count is what stops parallel guesses all passing a check + # none of them had incremented yet. + from unittest.mock import patch + + from apps.accounts import totp + from apps.accounts.models import TotpAttempt + seen = [] + real = totp.verify_totp + with patch.object(totp, "verify_totp", + side_effect=lambda s, c: seen.append(TotpAttempt.objects.filter(user=self.user).count()) + or real(s, c)): + consume_totp(self.user, self._wrong()) + self.assertEqual(seen, [1]) + + +class TotpDisableLimitTests(TestCase): + """Disabling TOTP used verify_totp directly: unlimited guesses at the one + action that lets a new device be enrolled afterwards.""" + + def setUp(self): + self.user = get_user_model().objects.create_user("d", password="pw") + self.profile = UserProfile.objects.create(user=self.user) + self.secret = generate_secret() + self.profile.totp_secret = self.secret + self.profile.totp_confirmed_at = now() + self.profile.save() + self.client.force_login(self.user) + + def test_disable_is_rate_limited(self): + good = generate_totp(self.secret) + wrong = "000000" if good != "000000" else "111111" + for _ in range(MAX_TOTP_FAILURES): + self.client.post("/api/v1/accounts/totp/disable/", {"code": wrong}, content_type="application/json") + resp = self.client.post("/api/v1/accounts/totp/disable/", {"code": good}, content_type="application/json") + self.assertEqual(resp.status_code, 400) + self.profile.refresh_from_db() + self.assertIsNotNone(self.profile.totp_confirmed_at, "TOTP must still be enrolled") diff --git a/server/apps/accounts/totp.py b/server/apps/accounts/totp.py index b16e1dae..14807cc3 100644 --- a/server/apps/accounts/totp.py +++ b/server/apps/accounts/totp.py @@ -74,6 +74,35 @@ def otpauth_uri(secret_b32: str, account_name: str, issuer: str = "Vigil") -> st _REPLAY_WINDOW_SECONDS = (2 * 1 + 1) * _STEP_SECONDS +#: Wrong codes allowed per user inside the window before TOTP-gated actions +#: are refused for the rest of it (architect review, 2026-10-08). +MAX_TOTP_FAILURES = 5 +TOTP_FAILURE_WINDOW_MINUTES = 10 + + +def _totp_attempt_allowed(user) -> bool: + """Record this attempt, then say whether it is within the limit. + + Recorded first and counted after, so requests racing each other each see + the others' rows: checking and recording in the other order let a burst of + parallel guesses all pass a check that none of them had yet incremented. + """ + from datetime import timedelta + + from django.utils.timezone import now + + from .models import TotpAttempt + TotpAttempt.objects.create(user=user) + since = now() - timedelta(minutes=TOTP_FAILURE_WINDOW_MINUTES) + TotpAttempt.objects.filter(created_at__lt=now() - timedelta(hours=24)).delete() + return TotpAttempt.objects.filter(user=user, created_at__gte=since).count() <= MAX_TOTP_FAILURES + + +def _clear_totp_attempts(user) -> None: + from .models import TotpAttempt + TotpAttempt.objects.filter(user=user).delete() + + def consume_totp(user, code: str) -> tuple[bool, str | None]: """Verify a TOTP code for ``user`` and mark it consumed. @@ -93,8 +122,13 @@ def consume_totp(user, code: str) -> tuple[bool, str | None]: code = (code or "").strip().replace(" ", "") if not code: return False, "TOTP code required" + # A six-digit code falls to guessing without a limit, and the gate exists + # for exactly the case where someone holds a session but not the device. + if not _totp_attempt_allowed(user): + return False, "Too many wrong codes — wait a few minutes and try again" if not verify_totp(secret, code): return False, "Invalid TOTP code" + _clear_totp_attempts(user) if profile.last_totp_code == code and profile.last_totp_used_at: elapsed = (now() - profile.last_totp_used_at).total_seconds() diff --git a/server/apps/accounts/urls.py b/server/apps/accounts/urls.py index a0653659..56caed63 100644 --- a/server/apps/accounts/urls.py +++ b/server/apps/accounts/urls.py @@ -12,4 +12,6 @@ path("totp/enroll/confirm/", views.totp_enroll_confirm, name="totp-enroll-confirm"), path("totp/disable/", views.totp_disable, name="totp-disable"), path("totp/debug-code/", views.totp_debug_code, name="totp-debug-code"), + path("session/activity/", views.session_activity, name="session-activity"), + path("session/", views.session_status, name="session-status"), ] diff --git a/server/apps/accounts/views.py b/server/apps/accounts/views.py index 13e4dfe6..9c218c19 100644 --- a/server/apps/accounts/views.py +++ b/server/apps/accounts/views.py @@ -107,8 +107,14 @@ def totp_disable(request): code = (request.data.get("code") or "").strip() if not profile.totp_confirmed_at or not profile.totp_secret: return Response({"error": "TOTP is not enrolled"}, status=400) - if not verify_totp(profile.totp_secret, code): - return Response({"error": "Invalid code"}, status=400) + # Through consume_totp, not verify_totp: turning TOTP off is the strongest + # thing a code unlocks (afterwards a new device can be enrolled), so it + # gets the same attempt limit and replay check as every other gate. + from .totp import consume_totp + ok, err = consume_totp(request.user, code) + if not ok: + return Response({"error": err or "Invalid code"}, status=400) + profile.refresh_from_db() profile.totp_secret = "" profile.totp_confirmed_at = None profile.save(update_fields=["totp_secret_encrypted", "totp_confirmed_at"]) @@ -292,6 +298,27 @@ def logout_view(request): return redirect("login") +@api_view(["POST"]) +@permission_classes([IsAuthenticated]) +def session_activity(request): + """The browser's heartbeat (QA-14). Sent only on real interaction. + + The middleware already refreshed ``last_active`` — this path is on the + activity list — so the view just reports what that bought. + """ + from .session_timeout import remaining + return Response(remaining(request.session)) + + +@api_view(["GET"]) +@permission_classes([IsAuthenticated]) +def session_status(request): + """Time left on the session, for the countdown. A background GET: checked, + never refreshing, or an open tab would sit out the idle clock forever.""" + from .session_timeout import remaining + return Response(remaining(request.session)) + + @api_view(["POST"]) @permission_classes([IsAuthenticated]) def totp_debug_code(request): diff --git a/server/apps/agent_dist/test_install_sec3.py b/server/apps/agent_dist/test_install_sec3.py new file mode 100644 index 00000000..203b3d81 --- /dev/null +++ b/server/apps/agent_dist/test_install_sec3.py @@ -0,0 +1,91 @@ +"""SEC-3: install.sh writes the server's key into agent.yml, keeps agent.yml +root-owned, and does not believe a task-running mode from a file the +unprivileged service account could have written. The block is run for real, +in bash, against a scratch copy of agent.yml.""" +import os +import re +import subprocess +import tempfile +from pathlib import Path + +from django.template.loader import render_to_string +from django.test import SimpleTestCase + +KEY = "q1Ivm7G1oQ0kq5mH0q2N4u3mXyZ9b8F6c5d4e3f2a1A=" +START = "# VIGIL_TOKEN ends up inside sed replacements" +END = "# ── Service installation" + + +class InstallShSec3Tests(SimpleTestCase): + def setUp(self): + script = render_to_string("agent_install.sh", {"base_url": "https://vigil.test", "public_key": KEY}) + self.block = script[script.index(START):script.index(END)] + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + self.cfg = Path(self.tmp.name) / "agent.yml" + + def _run(self, config, env=None): + if config is None: + self.cfg.unlink(missing_ok=True) + else: + self.cfg.write_text(config) + self.cfg.chmod(0o600) + body = (self.block.replace("/etc/vigil/agent.yml", str(self.cfg)) + .replace("/etc/vigil", str(self.cfg.parent))) + # chown/chmod to root are the real installer's job; here the file is ours. + harness = ('VIGIL_SERVER="https://vigil.test"\nchown() { :; }\nchmod() { :; }\n' + + body + '\necho "MODE=$CFG_MODE"\n') + result = subprocess.run(["bash", "-c", harness], capture_output=True, text=True, + env={**os.environ, **(env or {})}, timeout=30) + return result, (self.cfg.read_text() if self.cfg.exists() else "") + + def test_fresh_install_gets_the_key_and_monitor_mode(self): + result, text = self._run(None) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(text.count("server_public_key:"), 1) + self.assertIn(f'server_public_key: "{KEY}"', text) + self.assertRegex(text, r"(?m)^mode: monitor$") + self.assertIn('server_url: "https://vigil.test"', text) + + def test_untrusted_config_is_set_aside_and_rebuilt(self): + if os.geteuid() == 0: + self.skipTest("the scratch file would be root-owned") + planted = ('server_url: "https://evil.example"\nagent_token: "keepme-0123456789abcdef"\n' + 'mode: full_control\n"mode": full_control\nallowlist: [run_command]\n') + result, text = self._run(planted) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("none of its", result.stderr) + self.assertNotIn("evil.example", text) + self.assertNotIn("run_command", text) + self.assertNotIn("full_control", text) + self.assertIn("keepme-0123456789abcdef", text) + self.assertRegex(text, r"(?m)^mode: monitor$") + self.assertTrue(list(self.cfg.parent.glob("agent.yml.untrusted.*"))) + + def test_an_untrusted_token_that_is_not_a_token_is_dropped(self): + if os.geteuid() == 0: + self.skipTest("the scratch file would be root-owned") + result, text = self._run('server_url: "x"\nagent_token: "a|b&c;rm -rf /"\nmode: monitor\n') + self.assertEqual(result.returncode, 0, result.stderr) + self.assertNotIn("rm -rf", text) + self.assertIn("not a valid token", result.stderr) + + def test_vigil_mode_sets_the_mode(self): + result, text = self._run(None, {"VIGIL_MODE": "managed"}) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertRegex(text, r"(?m)^mode: managed$") + + def test_vigil_mode_and_token_are_validated(self): + self.assertNotEqual(self._run(None, {"VIGIL_MODE": "root"})[0].returncode, 0) + self.assertNotEqual(self._run(None, {"VIGIL_TOKEN": "x|y"})[0].returncode, 0) + + def test_monitor_config_is_root_owned_and_group_readable(self): + script = render_to_string("agent_install.sh", {"base_url": "https://vigil.test", "public_key": KEY}) + self.assertIn("chown root:vigil-agent /etc/vigil/agent.yml", script) + self.assertIn("chmod 640 /etc/vigil/agent.yml", script) + self.assertNotIn("chown vigil-agent /etc/vigil/agent.yml", script) + self.assertIsNone(re.search(r"AGENT_MODE=\"\$\(sed", script), "mode must come from CFG_MODE") + + def test_a_reinstall_restarts_a_running_agent(self): + script = render_to_string("agent_install.sh", {"base_url": "https://vigil.test", "public_key": KEY}) + self.assertIn('if [ -n "${VIGIL_TOKEN:-}" ] || systemctl is-active --quiet vigil-agent; then', script) diff --git a/server/apps/agent_dist/test_uninstall.py b/server/apps/agent_dist/test_uninstall.py index 254a0f14..b0b260af 100644 --- a/server/apps/agent_dist/test_uninstall.py +++ b/server/apps/agent_dist/test_uninstall.py @@ -95,3 +95,75 @@ def test_they_do_not_require_authentication(self): longer have working credentials.""" for name in ("agent-uninstall-script", "agent-uninstall-ps1"): self.assertEqual(self.client.get(reverse(name)).status_code, 200) + + +class WindowsConfigTreeLockedTests(SimpleTestCase): + """SEC-2: C:\\ProgramData's default ACL let a standard user create the scripts + folder the agent runs scripts from as LocalSystem (proven on the Windows VM). + The installer takes the tree back and locks it: fail closed, never following a + link a user planted (both checked live on the VM).""" + + def setUp(self): + self.ps1 = render_to_string("agent_install.ps1", {"base_url": BASE_URL, "public_key": "K"}) + + def _at(self, needle): + self.assertIn(needle, self.ps1) + return self.ps1.index(needle) + + def test_top_folder_is_locked_first_by_sid_without_following_links(self): + take = self._at('Invoke-AclStep $ConfigDir /setowner $Admins /L') + lock = self._at('Invoke-AclStep $ConfigDir /inheritance:r /grant "*S-1-5-18:(OI)(CI)(F)" ' + '/grant "$($Admins):(OI)(CI)(F)" /grant "*S-1-5-32-545:(RX)" /L') + self.assertLess(take, lock) + + def test_the_tree_is_walked_top_down_never_with_icacls_recursion(self): + # icacls /T follows directory junctions even with /L (measured on the VM). + walk = self.ps1[self.ps1.index("function Lock-Tree"):self.ps1.index("# The folder itself may be a link")] + self.assertNotIn("/T", walk) + self.assertIn("ReparsePoint", walk) + self.assertIn("[System.IO.Directory]::Delete($child.FullName)", walk) + self.assertLess(walk.index("/setowner $Admins /L"), walk.index("$pending.Push($child.FullName)"), + "a folder is taken over before it is listed") + config_steps = [line for line in self.ps1.splitlines() + if "$ConfigDir" in line and "icacls" in line.lower() and "/T" in line] + self.assertEqual(config_steps, []) + + def test_the_walk_runs_after_the_top_lock_and_before_the_specific_grants(self): + walk = self._at("Lock-Tree $ConfigDir\n") + self.assertLess(self._at("Invoke-AclStep $ConfigDir /inheritance:r"), walk) + self.assertLess(walk, self._at("Invoke-AclStep $ConfigPath /inheritance:r")) + + def test_a_planted_config_folder_link_is_replaced_before_use(self): + check = self._at("if (Test-Link $ConfigDir)") + self.assertLess(check, self._at("New-Item -ItemType Directory -Force -Path $ConfigDir")) + self.assertLess(check, self._at("New-Item -ItemType Directory -Force -Path $DataDir")) + + def test_every_lock_step_fails_closed(self): + self.assertIn("if ($LASTEXITCODE -ne 0) {", self.ps1) + self.assertIn("throw \"Could not secure", self.ps1) + block = self.ps1[self.ps1.index("function Invoke-AclStep"):self.ps1.index("New-Item -ItemType Directory -Force -Path $DataDir")] + self.assertNotIn("& icacls.exe $ConfigDir", block, "lock steps go through Invoke-AclStep") + + def test_scripts_folder_is_created_after_the_lock(self): + self.assertLess(self._at('Invoke-AclStep $ConfigDir /inheritance:r'), + self._at("New-Item -ItemType Directory -Force -Path $ScriptsDir")) + + def test_untrusted_scripts_are_quarantined_inside_the_walk(self): + walk = self.ps1[self.ps1.index("function Lock-Tree"):self.ps1.index("$Quarantine = Join-Path")] + # Judged after their folder is locked, by owner and by who may write. + self.assertIn("Test-Untrusted $child.FullName", walk) + self.assertLess(walk.index("$pending.Push($child.FullName)"), len(walk)) + trust = self.ps1[self.ps1.index("function Test-Untrusted"):self.ps1.index("function Move-ToQuarantine")] + self.assertIn("GetOwner($sid)", trust) + self.assertIn("FileSystemRights -band $WriteMask", trust) + move = self.ps1[self.ps1.index("function Move-ToQuarantine"):self.ps1.index("function Lock-Tree")] + self.assertIn("Substring($ScriptsDir.Length)", move, "keeps the relative path") + self.assertIn("Remove-Item -LiteralPath $Path -Force", move, "deletes what cannot be moved") + self.assertLess(self._at("$Quarantine = Join-Path"), self._at("Lock-Tree $ConfigDir\n")) + + def test_monitor_service_can_still_write_its_log(self): + self.assertIn('Invoke-AclStep $LogPath /grant "$($ServiceAccount):(M)" /L', self.ps1) + + def test_server_key_is_written_into_agent_yml(self): + self.assertIn('$ServerPublicKey = "K"', self.ps1) + self.assertIn("server_public_key:", self.ps1) diff --git a/server/apps/agent_dist/views.py b/server/apps/agent_dist/views.py index ce496dc2..48797872 100644 --- a/server/apps/agent_dist/views.py +++ b/server/apps/agent_dist/views.py @@ -1,4 +1,5 @@ import hashlib +from vigil.signing import get_public_key_b64 from pathlib import Path from django.conf import settings @@ -156,14 +157,14 @@ def uninstall_ps1(request): def install_script(request): """Return the bash installer for Linux and macOS.""" base_url = f"{request.scheme}://{request.get_host()}" - content = render_to_string("agent_install.sh", {"base_url": base_url}) + content = render_to_string("agent_install.sh", {"base_url": base_url, "public_key": get_public_key_b64()}) return HttpResponse(content, content_type="text/plain; charset=utf-8") def install_ps1(request): """Return the PowerShell installer for Windows.""" base_url = f"{request.scheme}://{request.get_host()}" - content = render_to_string("agent_install.ps1", {"base_url": base_url}) + content = render_to_string("agent_install.ps1", {"base_url": base_url, "public_key": get_public_key_b64()}) return HttpResponse(content, content_type="text/plain; charset=utf-8") diff --git a/server/apps/automations/tests.py b/server/apps/automations/tests.py index e0b14789..8cc56c40 100644 --- a/server/apps/automations/tests.py +++ b/server/apps/automations/tests.py @@ -347,7 +347,7 @@ def test_the_run_resolves_when_the_task_reports_back(self): data=json.dumps({"task_id": str(task.id), "state": "completed", "output": "done"}), content_type="application/json", - HTTP_AUTHORIZATION=f"Bearer {host.agent_token}") + HTTP_AUTHORIZATION=f"Bearer {host.raw_agent_token}") self.assertIn(resp.status_code, (200, 201), resp.content) run = TaskRun.objects.get() self.assertEqual(run.state, TaskRun.State.COMPLETED) diff --git a/server/apps/civilsso/tests.py b/server/apps/civilsso/tests.py index d92544d3..ac92605a 100644 --- a/server/apps/civilsso/tests.py +++ b/server/apps/civilsso/tests.py @@ -117,6 +117,12 @@ def test_inactive_local_user_cannot_enter(self): class DisabledTests(TestCase): + def setUp(self): + # With no account at all, SetupRedirectMiddleware sends every page to + # /setup/ — unless an earlier test in the same process already set its + # once-per-process flag. A user makes this test about Civil, not order. + get_user_model().objects.create_user("someone", password="x") + def test_unconfigured_means_404(self): # No CIVIL_URL: the routes simply do not exist for this install. self.assertEqual( @@ -127,6 +133,12 @@ def test_unconfigured_means_404(self): @override_settings(CIVIL_URL="http://civil.test") class LoginStartTests(TestCase): + def setUp(self): + # An account exists, as on any installed Vigil: otherwise + # SetupRedirectMiddleware sends the page to /setup/, unless an earlier + # test in the same process already set its once-per-process flag. + get_user_model().objects.create_user("someone", password="x") + def test_redirects_to_civil_with_state_and_callback(self): resp = self.client.get("/accounts/civil/login/", {"next": "/"}) self.assertEqual(resp.status_code, 302) diff --git a/server/apps/hosts/authentication.py b/server/apps/hosts/authentication.py index 0607465d..41b94a2c 100644 --- a/server/apps/hosts/authentication.py +++ b/server/apps/hosts/authentication.py @@ -3,16 +3,25 @@ from .models import Host -def authenticate_agent(request): +def authenticate_agent(request, allow_unapproved: bool = False): """Validate a Bearer agent token. Returns (Host, None) on success or (None, error Response) on failure. + + Only an approved host passes unless *allow_unapproved*. Registering is open + by design, so a token alone proves nothing until an admin approves the host: + without this, a host nobody approved could fetch registry credentials and + use every other agent endpoint. Check-in is the one caller that opts out, + since a pending host has to check in to reach the enrolment queue. """ header = request.META.get("HTTP_AUTHORIZATION", "") if not header.startswith("Bearer "): return None, Response({"error": "Agent token required"}, status=401) token = header[7:].strip() try: - return Host.objects.get(agent_token=token), None + host = Host.objects.by_token(token).get() except Host.DoesNotExist: - return None, Response({"error": "Invalid agent token"}, status=401) \ No newline at end of file + return None, Response({"error": "Invalid agent token"}, status=401) + if not allow_unapproved and host.status in (Host.Status.PENDING, Host.Status.REJECTED): + return None, Response({"error": "This host has not been approved"}, status=403) + return host, None \ No newline at end of file diff --git a/server/apps/hosts/firewall_guard.py b/server/apps/hosts/firewall_guard.py index b12a7d12..aedc9d6e 100644 --- a/server/apps/hosts/firewall_guard.py +++ b/server/apps/hosts/firewall_guard.py @@ -11,6 +11,11 @@ matters -- so a host on a custom SSH port is simply not protected here, and this is a deliberate, documented limitation. +Removing a ``deny``/``reject`` rule on a protected port is always allowed: +blocking a port is what cuts access off, and undoing that can only restore +it. Refusing such a removal is what would strand a host whose SSH had been +blocked -- the rule is the way back in. + ``check_change`` must never raise. Task 9's endpoint calls it before doing anything else, so a malformed or partial snapshot -- missing keys, an ``unparsed`` bucket instead of a parsed rule, ``"unknown"`` defaults, a @@ -46,6 +51,10 @@ _LOCKOUT_POLICIES = ("deny", "reject") +#: A rule disposition that blocks traffic. Removing one can only restore +#: access; adding one on a protected port is how a lockout is done. +_BLOCKING = ("deny", "reject") + def _protected_ports(host, snapshot: dict) -> set[int]: ports = {SSH_PORT} @@ -63,6 +72,33 @@ def _as_int(value) -> int | None: return None +def _is_known_block(snapshot, params, disposition) -> bool: + """True only when the snapshot holds this exact rule as a block. + + The claimed action comes from the client, and Windows removes by rule_id + regardless of action -- so a claimed "deny" carrying the RDP *allow* + rule's id must not pass. With a rule_id, the snapshot rule with that + rule_id must carry a blocking action. Without one, a rule with the same + port, a protocol matching the requested one (or "any"), and that same + blocking action must exist. + """ + raw = snapshot.get("rules") + rules = [r for r in (raw if isinstance(raw, list) else []) if isinstance(r, dict)] + rule_id = str(params.get("rule_id") or "").strip() + port = _as_int(params.get("port")) + proto = str(params.get("protocol") or "").strip().lower() + for r in rules: + r_action = str(r.get("action", "")).strip().lower() + if rule_id: + if str(r.get("rule_id") or "").strip() == rule_id: + return r_action in _BLOCKING + continue + if (_as_int(r.get("port")) == port and r_action == disposition + and str(r.get("protocol", "")).strip().lower() in (proto, "any")): + return True + return False + + def _params(change: dict) -> dict: params = change.get("params") return params if isinstance(params, dict) else {} @@ -108,23 +144,29 @@ def check_change(host, snapshot, change) -> str: if action == "add_firewall_rule": port = _as_int(params.get("port")) disposition = str(params.get("action", "allow")).strip().lower() - if disposition == "deny": + if disposition in _BLOCKING: if port is None: return ( - "Refusing to deny an unparseable port -- the guard " - "cannot confirm this is not one of the ports that " + f"Refusing to {disposition} an unparseable port -- the " + "guard cannot confirm this is not one of the ports that " "reach this host (22, or 3389 on Windows). Write it as " "a task by hand if you really mean it.") if port in protected: return ( - f"Refusing to deny port {port} -- that is how this " - f"host is reached, and Vigil could not undo it " + f"Refusing to {disposition} port {port} -- that is how " + f"this host is reached, and Vigil could not undo it " f"remotely. Write it as a task by hand if you really " f"mean it.") return "" if action == "remove_firewall_rule": port = _as_int(params.get("port")) + disposition = str(params.get("action", "allow")).strip().lower() + if disposition in _BLOCKING and _is_known_block(snapshot, params, + disposition): + # Removing a block re-opens access; it can never lock anyone out. + # Refusing it is what would strand a host whose SSH was blocked. + return "" if port is None: return ( "Refusing to remove a rule for an unparseable port -- the " diff --git a/server/apps/hosts/migrations/0025_host_agent_runs_as_root.py b/server/apps/hosts/migrations/0025_host_agent_runs_as_root.py new file mode 100644 index 00000000..1476191a --- /dev/null +++ b/server/apps/hosts/migrations/0025_host_agent_runs_as_root.py @@ -0,0 +1,18 @@ +# Generated by Django 5.2.17 on 2026-10-07 00:29 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('hosts', '0024_merge_0018_host_machine_id_0023_container_rollback'), + ] + + operations = [ + migrations.AddField( + model_name='host', + name='agent_runs_as_root', + field=models.BooleanField(blank=True, null=True), + ), + ] diff --git a/server/apps/hosts/migrations/0026_hash_agent_tokens.py b/server/apps/hosts/migrations/0026_hash_agent_tokens.py new file mode 100644 index 00000000..ca71ece2 --- /dev/null +++ b/server/apps/hosts/migrations/0026_hash_agent_tokens.py @@ -0,0 +1,29 @@ +# Generated by Django 5.2.17 on 2026-10-08 18:39 + +import hashlib + +from django.db import migrations + +PREFIX = "sha256$" + + +def hash_tokens(apps, schema_editor): + """Store every agent token as a SHA-256 (architect review, 2026-10-08). + Agents keep presenting their plaintext token; the server hashes it to look + the host up, so nothing changes for a running agent.""" + Host = apps.get_model("hosts", "Host") + for host in Host.objects.exclude(agent_token__startswith=PREFIX).only("id", "agent_token"): + host.agent_token = PREFIX + hashlib.sha256(host.agent_token.encode()).hexdigest() + host.save(update_fields=["agent_token"]) + + +class Migration(migrations.Migration): + + dependencies = [ + ('hosts', '0025_host_agent_runs_as_root'), + ] + + operations = [ + # One way: a hash cannot be turned back into the token. + migrations.RunPython(hash_tokens, migrations.RunPython.noop), + ] diff --git a/server/apps/hosts/models.py b/server/apps/hosts/models.py index a725d440..5368c23f 100644 --- a/server/apps/hosts/models.py +++ b/server/apps/hosts/models.py @@ -1,10 +1,34 @@ +import hashlib +import re import uuid from django.conf import settings from django.db import models +#: Agent tokens are kept as a SHA-256, never in the clear: a database dump must +#: not hand out a working credential for every machine (architect review, +#: 2026-10-08). The hex part doubles as the agent fingerprint that v2 task +#: signatures bind to (vigil/signing.py), since the agent hashes its own token. +TOKEN_PREFIX = "sha256$" +#: The shape a token presented at registration or enrolment must have. +TOKEN_RE = re.compile(r"^[A-Za-z0-9_-]{16,128}$") + + +def hash_agent_token(raw: str) -> str: + """The stored form of a plaintext token. Always hashes: a caller holding a + stored value must not be able to present it as if it were the token.""" + return TOKEN_PREFIX + hashlib.sha256(str(raw).encode()).hexdigest() + + +class HostManager(models.Manager): + def by_token(self, raw: str): + """Hosts whose token is *raw* (the plaintext an agent presents).""" + return self.filter(agent_token=hash_agent_token(raw)) + class Host(models.Model): + objects = HostManager() + class Status(models.TextChoices): PENDING = "pending", "Pending Enrollment" ONLINE = "online", "Online" @@ -39,10 +63,23 @@ class Mode(models.TextChoices): tag_sync_fields = [("tags", "tag_rows")] def save(self, *args, **kwargs): + # A plaintext token set anywhere (registration, enrolment, tests) is + # stored hashed; one that already is stays as it is. + if self.agent_token and not self.agent_token.startswith(TOKEN_PREFIX): + # The plaintext stays on this in-memory instance only (never stored, + # never read by the server) so whoever just set it, a test or an + # enrolment flow, can still hand it to the agent. + self.raw_agent_token = self.agent_token + self.agent_token = hash_agent_token(self.agent_token) super().save(*args, **kwargs) # Defined below this class, hence the local import. for string_field, relation in self.tag_sync_fields: sync_tag_rows(self, string_field, relation) + @property + def token_fingerprint(self) -> str: + """SHA-256 hex of the agent's token: what a v2 signature binds to.""" + return self.agent_token[len(TOKEN_PREFIX):] if self.agent_token.startswith(TOKEN_PREFIX) else "" + agent_version = models.CharField(max_length=50, blank=True, default="") #: Task-language features the agent said it understands at check-in. #: The server refuses to hand a task to a host that lacks a feature the @@ -53,6 +90,9 @@ def save(self, *args, **kwargs): # agent new enough to report it checks in — unknown, which is not "refuses". agent_allowlist = models.JSONField(null=True, blank=True) agent_allow_reprovision = models.BooleanField(default=False) + # Whether the agent's process runs as root, as it last reported it (QA-08): + # None until an agent new enough to report it checks in. + agent_runs_as_root = models.BooleanField(null=True, blank=True) last_checkin = models.DateTimeField(null=True, blank=True) #: What this machine is missing, as the agent last counted it: #: {"pending", "critical", "important", "reboot_required"}. Null means no diff --git a/server/apps/hosts/serializers.py b/server/apps/hosts/serializers.py index 6d9ffe52..e54e4a78 100644 --- a/server/apps/hosts/serializers.py +++ b/server/apps/hosts/serializers.py @@ -27,6 +27,7 @@ class Meta: "agent_version", "agent_allowlist", "agent_allow_reprovision", + "agent_runs_as_root", "reboot_required", "last_checkin", "created_at", diff --git a/server/apps/hosts/test_agent_auth_approval.py b/server/apps/hosts/test_agent_auth_approval.py new file mode 100644 index 00000000..cb9064e9 --- /dev/null +++ b/server/apps/hosts/test_agent_auth_approval.py @@ -0,0 +1,47 @@ +"""Only an approved host may use the agent endpoints that hand out secrets or +change state (architect review, 2026-10-08). Registering is open by design, so +before this a host nobody had approved could fetch every registry credential +scoped to "all hosts" just by registering. Check-in alone still accepts a +pending host: that is how it reaches the enrolment queue.""" +from django.test import TestCase + +from apps.hosts.models import Host +from apps.stacks.models import RegistryCredential +from vigil.signing import get_signing_key # noqa: F401 (settings import order) + + +def _cred(): + from apps.hosts.crypto import encrypt_secret + return RegistryCredential.objects.create(registry="ghcr.io", username="bot", + password_encrypted=encrypt_secret("s3cret"), host_tags=[]) + + +class AgentAuthApprovalTests(TestCase): + def _get(self, token): + return self.client.get("/api/v1/agent/registry-auth/?registry=ghcr.io", + HTTP_AUTHORIZATION=f"Bearer {token}") + + def test_a_pending_host_gets_no_credentials(self): + _cred() + Host.objects.create(hostname="new", agent_token="tok-new", status=Host.Status.PENDING) + resp = self._get("tok-new") + self.assertEqual(resp.status_code, 403) + self.assertNotIn("s3cret", resp.content.decode()) + + def test_a_rejected_host_gets_no_credentials(self): + _cred() + Host.objects.create(hostname="bad", agent_token="tok-bad", status=Host.Status.REJECTED) + self.assertEqual(self._get("tok-bad").status_code, 403) + + def test_an_approved_host_still_does(self): + _cred() + Host.objects.create(hostname="ok", agent_token="tok-ok", status=Host.Status.ONLINE) + resp = self._get("tok-ok") + self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.json()["password"], "s3cret") + + def test_a_pending_host_can_still_check_in(self): + Host.objects.create(hostname="new", agent_token="tok-new", status=Host.Status.PENDING) + resp = self.client.post("/api/v1/checkin", {"hostname": "new", "metrics": {}}, + content_type="application/json", HTTP_AUTHORIZATION="Bearer tok-new") + self.assertNotEqual(resp.status_code, 403) diff --git a/server/apps/hosts/test_container_states.py b/server/apps/hosts/test_container_states.py new file mode 100644 index 00000000..ef01da8f --- /dev/null +++ b/server/apps/hosts/test_container_states.py @@ -0,0 +1,71 @@ +"""Stopped and crashed containers and stacks stay visible, with their exit code. + +User, 2026-10-09: "make sure stack that crashed or stop are visible as stopped +or exited code 0". Docker's status text carries the code ("Exited (137) …"); +the page showed only "exited", so a clean stop and a crash looked the same, +and a managed stack with no containers left vanished from the list. +""" + +import re +import shutil +import subprocess +from pathlib import Path +from unittest import skipUnless + +from django.test import SimpleTestCase + +ROOT = Path(__file__).resolve().parents[2] +JS = ROOT / "static/js/vigil-containers.js" + + +def _function(src: str, name: str) -> str: + start = src.index(f"function {name}(") + depth, i = 0, src.index("{", start) + while True: + depth += {"{": 1, "}": -1}.get(src[i], 0) + if depth == 0: + return src[start:i + 1] + i += 1 + + +class ContainerStateTests(SimpleTestCase): + def setUp(self): + self.src = JS.read_text() + + def test_rows_and_stacks_use_the_exit_aware_badges(self): + self.assertIn("${_ctrStateBadge(c)}", self.src) + self.assertIn("_stackStateBadge(rows)", self.src) + + def test_a_down_managed_stack_stays_listed(self): + self.assertRegex(self.src, r"downStacks = \(stacks \|\| \[\]\)\.filter\(s => s\.ownership !== 'external'") + self.assertIn("for (const s of downStacks)", self.src) + + @skipUnless(shutil.which("node"), "node is not installed") + def test_badges_tell_a_clean_stop_from_a_crash(self): + """Run the real functions from the file, not a copy of them.""" + code = "\n".join([ + "function escHtml(s){return String(s);} function escAttr(s){return String(s);}", + re.search(r"const CTR_STATES = \[[^\]]*\];", self.src).group(0), + _function(self.src, "_ctrExitCode"), + _function(self.src, "_ctrStateBadge"), + _function(self.src, "_stackStateBadge"), + "const C=(state,status)=>({state,status});", + "console.log(JSON.stringify([", + " _ctrStateBadge(C('exited','Exited (0) 2 hours ago')),", + " _ctrStateBadge(C('exited','Exited (137) 5 minutes ago')),", + " _ctrStateBadge(C('running','Up 3 hours')),", + " _stackStateBadge([C('exited','Exited (0) 1 hour ago')]),", + " _stackStateBadge([C('running','Up'), C('exited','Exited (1) now')]),", + " _stackStateBadge([C('dead','Dead')]),", + "]));", + ]) + out = subprocess.run(["node", "-e", code], capture_output=True, text=True, timeout=30, check=True) + clean, crash, running, stack_stopped, stack_partial, stack_dead = __import__("json").loads(out.stdout) + self.assertIn("stopped (exit 0)", clean) + self.assertIn('class="ctr-state stopped"', clean) + self.assertIn("crashed (exit 137)", crash) + self.assertIn('class="ctr-state exited"', crash) + self.assertIn(">running<", running) + self.assertIn("stopped (exit 0)", stack_stopped) + self.assertIn("1/2 running · 1 crashed (exit 1)", stack_partial) + self.assertIn("crashed", stack_dead) diff --git a/server/apps/hosts/test_devices.py b/server/apps/hosts/test_devices.py index 5e638452..cd272bbe 100644 --- a/server/apps/hosts/test_devices.py +++ b/server/apps/hosts/test_devices.py @@ -2,6 +2,7 @@ from django.test import TestCase from rest_framework.test import APIClient +from apps.accounts.models import Role, UserProfile from apps.hosts.models import UnmanagedDevice @@ -9,6 +10,8 @@ class UnmanagedDeviceApiTests(TestCase): def setUp(self): self.client = APIClient() self.user = get_user_model().objects.create_user("op", password="pw") + # Adding a device is admin-only, like editing one (SEC-1). + UserProfile.objects.create(user=self.user, role=Role.ADMIN) self.client.force_authenticate(self.user) def test_create_and_list_device(self): diff --git a/server/apps/hosts/test_docker.py b/server/apps/hosts/test_docker.py index 1abb7f92..3212b350 100644 --- a/server/apps/hosts/test_docker.py +++ b/server/apps/hosts/test_docker.py @@ -32,7 +32,7 @@ def setUp(self): def _checkin(self, body): return self.client.post( "/api/v1/checkin", body, format="json", - HTTP_AUTHORIZATION=f"Bearer {self.host.agent_token}", + HTTP_AUTHORIZATION=f"Bearer {self.host.raw_agent_token}", ) def test_checkin_ingests_docker_containers(self): diff --git a/server/apps/hosts/test_enrollment_replacement.py b/server/apps/hosts/test_enrollment_replacement.py index 1ef9528f..f20b8cf8 100644 --- a/server/apps/hosts/test_enrollment_replacement.py +++ b/server/apps/hosts/test_enrollment_replacement.py @@ -131,7 +131,7 @@ def test_a_second_pending_row_for_the_same_machine_is_cleared(self): def test_check_pending_reports_what_it_replaces(self): resp = self.client.post( "/api/v1/hosts/check-pending/", - {"token": self.host_b.agent_token}, + {"token": self.host_b.raw_agent_token}, format="json", ) self.assertEqual(resp.status_code, 200) @@ -149,7 +149,7 @@ def test_check_pending_reports_null_when_nothing_is_replaced(self): ) resp = self.client.post( "/api/v1/hosts/check-pending/", - {"token": solo.agent_token}, + {"token": solo.raw_agent_token}, format="json", ) self.assertEqual(resp.status_code, 200) diff --git a/server/apps/hosts/test_firewall_guard.py b/server/apps/hosts/test_firewall_guard.py index 53fc9e2f..80f819a4 100644 --- a/server/apps/hosts/test_firewall_guard.py +++ b/server/apps/hosts/test_firewall_guard.py @@ -3,8 +3,16 @@ The task editor stays unguarded on purpose — that is the escape hatch. This guard exists so a form cannot do by accident what a hand-written task can do deliberately. + +Rule actions in a snapshot are `allow` / `deny` / `reject` (lowercase), and +the removal tests that name a specific rule use `snapshot_with()` to put that +rule in the snapshot the guard checks. `remove()` stays the port-only shape +so the pre-existing "a missing action means allow" tests keep their meaning; +`remove_rule()` adds the `rule_id` the Firewall tab sends. """ -from django.test import TestCase +from pathlib import Path + +from django.test import SimpleTestCase, TestCase from apps.hosts.firewall_guard import check_change @@ -13,20 +21,51 @@ "rules": [{"port": 22, "protocol": "tcp", "action": "allow", "source": "any", "interface": ""}]} +WINDOWS = {"tool": "windows", "enabled": True, + "defaults": {"incoming": "allow", "outgoing": "allow"}, + "rules": [{"port": 3389, "protocol": "tcp", "action": "allow", + "source": "any", "interface": "", "rule_id": "RDP-In"}]} + +DENY_22 = {"port": 22, "protocol": "tcp", "action": "deny", + "source": "10.0.0.5", "interface": "", "rule_id": ""} + +REJECT_22 = {**DENY_22, "action": "reject"} + + +def snapshot_with(base, *rules): + """A snapshot copy carrying extra rules, the way a real one does.""" + return {**base, "rules": list(base["rules"]) + [dict(r) for r in rules]} + class Host: def __init__(self, os="Ubuntu 24.04"): self.os = os +def windows(): + return Host(os="Windows Server 2022") + + def add(port, action="allow", protocol="tcp"): return {"action": "add_firewall_rule", "params": {"port": port, "protocol": protocol, "action": action}} -def remove(port, protocol="tcp"): - return {"action": "remove_firewall_rule", - "params": {"port": port, "protocol": protocol}} +def remove(port, protocol="tcp", action=None): + params = {"port": port, "protocol": protocol} + if action is not None: + params["action"] = action + return {"action": "remove_firewall_rule", "params": params} + + +def remove_rule(port, protocol="tcp", action=None, rule_id=None): + """A removal that names the rule, the way the Firewall tab does.""" + params = {"port": port, "protocol": protocol} + if action is not None: + params["action"] = action + if rule_id is not None: + params["rule_id"] = rule_id + return {"action": "remove_firewall_rule", "params": params} def policy(direction, value): @@ -35,14 +74,78 @@ def policy(direction, value): class LockoutGuardTests(TestCase): + def setUp(self): + self.host = Host() + def test_an_ordinary_rule_is_allowed(self): - self.assertEqual(check_change(Host(), LINUX, add(8080)), "") + self.assertEqual(check_change(self.host, LINUX, add(8080)), "") def test_denying_ssh_is_refused(self): - self.assertIn("22", check_change(Host(), LINUX, add(22, "deny"))) + r = check_change(self.host, LINUX, add(22, "deny")) + self.assertIn("22", r) + self.assertTrue(r) def test_removing_the_ssh_allow_rule_is_refused(self): - self.assertNotEqual(check_change(Host(), LINUX, remove(22)), "") + self.assertNotEqual(check_change(self.host, LINUX, remove(22)), "") + + # -- removing a *block* is the way back in ------------------------------ + + def test_removing_a_deny_on_ssh_is_allowed(self): + self.assertEqual( + check_change(self.host, snapshot_with(LINUX, DENY_22), + remove(22, action="deny")), + "") + + def test_removing_a_reject_on_ssh_is_allowed(self): + self.assertEqual( + check_change(self.host, snapshot_with(LINUX, REJECT_22), + remove(22, action="reject")), + "") + + def test_removing_a_block_on_rdp_is_allowed_on_windows(self): + self.assertEqual( + check_change( + windows(), + snapshot_with(WINDOWS, {"port": 3389, "protocol": "tcp", + "action": "reject", + "rule_id": "Block-RDP"}), + remove_rule(3389, action="reject", rule_id="Block-RDP")), + "") + + def test_removing_the_ssh_allow_is_still_refused_with_an_explicit_action(self): + self.assertNotEqual( + check_change(self.host, LINUX, remove(22, action="allow")), "") + + def test_a_claimed_deny_on_the_rdp_allow_is_refused(self): + """The action comes from the browser and Windows removes by rule_id, + so a claimed deny carrying the RDP *allow* rule's id is a lockout.""" + self.assertNotEqual( + check_change(windows(), WINDOWS, + remove_rule(3389, action="deny", rule_id="RDP-In")), + "") + + def test_a_block_missing_from_the_snapshot_is_refused(self): + """Nothing to remove, so nothing to confirm the claim against — fail + safe rather than wave an unverified action through.""" + self.assertNotEqual( + check_change(self.host, LINUX, remove(22, action="deny")), "") + + def test_removing_a_block_on_an_unprotected_port_is_allowed(self): + self.assertEqual( + check_change(self.host, LINUX, remove(8080, action="deny")), "") + + # -- adding a block ----------------------------------------------------- + + def test_rejecting_ssh_is_refused(self): + r = check_change(self.host, LINUX, add(22, "reject")) + self.assertTrue(r) + self.assertIn("22", r) + + def test_rejecting_rdp_is_refused_on_windows(self): + self.assertTrue(check_change(windows(), WINDOWS, add(3389, "reject"))) + + def test_rejecting_an_unparseable_port_is_refused(self): + self.assertTrue(check_change(self.host, LINUX, add("ssh", "reject"))) def test_denying_outbound_is_refused(self): """The worst one: agents are outbound-only, so this severs the agent's @@ -179,3 +282,34 @@ def test_denying_an_unparseable_port_is_refused(self): def test_removing_a_rule_for_a_non_numeric_port_is_refused(self): msg = check_change(Host(), LINUX, remove("ssh")) self.assertIn("unparseable", msg.lower()) + + +class FirewallUiMirrorTests(SimpleTestCase): + """The Firewall tab decides which rules get a Remove button by mirroring + this guard. When the two disagree the tab offers a change the server is + guaranteed to refuse, so the mirror is checked here rather than by eye.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.source = (Path(__file__).resolve().parents[2] + / "static" / "js" / "vigil-firewall.js").read_text( + encoding="utf-8") + + def test_the_protected_check_mirrors_the_allows_only_rule(self): + """Only deny/reject are removable on a protected port — the guard + refuses an allow, and a rule whose action could not be read.""" + self.assertIn("function _fwIsProtected(port, tool, action)", + self.source) + self.assertIn("if (action === 'deny' || action === 'reject') return false;", + self.source) + + def test_reject_rules_are_no_longer_marked_unremovable(self): + """The agent removes reject rules now, so the tab must offer the + button instead of a dead label.""" + self.assertNotIn("[cannot remove]", self.source) + + def test_the_protected_label_names_the_service_it_keeps_open(self): + for label in ("[protected — keeps SSH open]", + "[protected — keeps RDP open]"): + self.assertIn(label, self.source) diff --git a/server/apps/hosts/test_firewall_page.py b/server/apps/hosts/test_firewall_page.py new file mode 100644 index 00000000..c301bf10 --- /dev/null +++ b/server/apps/hosts/test_firewall_page.py @@ -0,0 +1,66 @@ +"""The Firewall page must open on a host, not on "Choose a host…". + +The host control used to be a , which ignores +