From ae5ead3e260a86efae31d739cd8475ffd8b5d2db Mon Sep 17 00:00:00 2001 From: Erin Spencer Date: Mon, 19 Oct 2026 19:36:19 -0700 Subject: [PATCH] fix(vm-contact): check d skips unsearchable dirs; exit 9 points to U6 Check d's vmmcp-writable find prunes directories vmmcp cannot enter (the root-only __pycache__), so their permission error no longer reads as READ-FAILED; any other find error still does. A runbook test runs the published find against a temp tree. The exit-9 note now says U4 has already stopped the units and points to U6, and a stray leading space is removed. --- backend/deploy/VM_CONTACT.md | 18 +++++++---- backend/tests/test_vm_contact_runbook.py | 38 ++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/backend/deploy/VM_CONTACT.md b/backend/deploy/VM_CONTACT.md index b67d1bf4..1e331de8 100644 --- a/backend/deploy/VM_CONTACT.md +++ b/backend/deploy/VM_CONTACT.md @@ -490,7 +490,9 @@ From #131 (`e74ce1e`) a rerun over an existing install is upgrade-safe: - Before anything is overwritten, the previous install is copied to a root-only directory under `/var/backups/vm-mcp/`; the installer prints the path. That is an extra copy alongside U1's backup, not a replacement for it. -- If that backup fails the installer exits 9 with nothing changed. +- If that backup fails the installer exits 9 before changing any installed file. U4 + has already stopped the units by then, so the MCP path is down: use U6 to restore + the prior activation. Exit order in the pinned installer: 3 (token, or a settings change without the override) and 7 (Yama precheck) come before the installer's backup; 9 means that @@ -753,7 +755,7 @@ The pinned `e74ce1e` is the merge of The-Interdependency/skill-lib#131 (upgrade- installer reruns, see U4), on top of #130 (`acd5e9f`, a first-use context notice from vm-mcp; its privileged components record a one-time marker in a new root-only state directory), #127 (`023ae12`) and #123 (`24029ca`). - #127 hardens the installer and makes the server check its token +#127 hardens the installer and makes the server check its token descriptor: fd 0 must be a regular file owned by root with no group or other bits. The installer's `/etc/credstore/vm-mcp-token` (root, `0600`) satisfies that. From #123 on, vm-mcp refuses every HTTP request, loopback included, that lacks @@ -1006,7 +1008,9 @@ false `PASS`. ```bash ( ok() { if [ "$2" = "$3" ]; then echo "PASS $1"; else echo "FAIL $1 (want $2, got $3)"; fi; } -w=$(sudo -n -u vmmcp find /opt/vm-mcp -writable) || w=READ-FAILED +# Directories vmmcp cannot search (root-only __pycache__) are pruned: nothing in them is +# reachable by vmmcp. Any other find error still sets READ-FAILED. +w=$(sudo -n -u vmmcp find /opt/vm-mcp -type d ! -executable -prune -o -writable -print) || w=READ-FAILED ok "nothing vmmcp-writable" 0 "$(grep -c . <<<"$w" || true)" o=$(sudo -n find /opt/vm-mcp ! -user root) || o=READ-FAILED ok "everything root-owned" 0 "$(grep -c . <<<"$o" || true)" @@ -1017,8 +1021,12 @@ stat -c '%U %a %n' /opt /opt/vm-mcp /opt/vm-mcp/.venv/pyvenv.cfg ``` Expect three `PASS` lines. All three paths must be owned by `root` with no group or -world write bit (for example `755` or `644`). If `find` reports a directory it can't read, the check -prints `FAIL`. Look at that directory's permissions before treating the result as a pass. +world write bit (for example `755` or `644`). The first check skips directories that +`vmmcp` cannot enter, such as the root-only `__pycache__`, because `vmmcp` can reach +nothing inside them; those directories themselves are not writable by `vmmcp` either. +If `find` still reports an error (for example a directory `vmmcp` can enter but not +list), the check prints `FAIL`. Look at that directory's permissions before treating +the result as a pass. ### e. HTTP authentication diff --git a/backend/tests/test_vm_contact_runbook.py b/backend/tests/test_vm_contact_runbook.py index fe13e5bf..bb59f893 100644 --- a/backend/tests/test_vm_contact_runbook.py +++ b/backend/tests/test_vm_contact_runbook.py @@ -8,8 +8,10 @@ from __future__ import annotations from pathlib import Path +import os import re import subprocess +import tempfile import unittest RUNBOOK = Path(__file__).resolve().parents[1] / "deploy" / "VM_CONTACT.md" @@ -103,5 +105,41 @@ def test_broker_receipt_check_only_counts_this_run(self): self.assertIn("CHECK_PORT, CHECK_PROFILE, CHECK_SOCKET, TOKEN_FILE, CHECK_ROOT = load_target()", block) + def _check_d_writable(self): + lines = [text for _, block in self.blocks for text in block.splitlines() + if text.startswith("w=$(sudo -n -u vmmcp find /opt/vm-mcp")] + self.assertEqual(len(lines), 1) + return lines[0] + + def test_check_d_prunes_unsearchable_dirs_and_keeps_read_failures(self): + line = self._check_d_writable() + self.assertTrue(line.endswith("|| w=READ-FAILED")) + self.assertIn("-type d ! -executable -prune -o -writable -print", line) + + @unittest.skipIf(os.geteuid() == 0, "permission bits do not bind root") + def test_check_d_find_behaviour(self): + line = self._check_d_writable().replace("sudo -n -u vmmcp ", "") + script = line.replace("/opt/vm-mcp", '"$1"') + '\nprintf "%s" "$w"\n' + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp, "opt") + (root / "__pycache__").mkdir(parents=True) + (root / "server.py").write_text("x") + (root / "server.py").chmod(0o444) + (root / "__pycache__").chmod(0o000) + run = lambda: subprocess.run(["bash", "-c", script, "_", str(root)], + text=True, capture_output=True, check=False).stdout + try: + root.chmod(0o555) + self.assertEqual("", run(), "an unsearchable dir must not read as a failure") + (root / "server.py").chmod(0o644) + self.assertEqual(str(root / "server.py"), run().strip()) + (root / "server.py").chmod(0o444) + (root / "__pycache__").chmod(0o100) # enterable but not listable + self.assertEqual("READ-FAILED", run()) + finally: + root.chmod(0o755) + (root / "__pycache__").chmod(0o755) + + if __name__ == "__main__": unittest.main()