diff --git a/skills/kvb-org-organize/scripts/nofollow.py b/skills/kvb-org-organize/scripts/nofollow.py index a5a385a..e62e6e5 100644 --- a/skills/kvb-org-organize/scripts/nofollow.py +++ b/skills/kvb-org-organize/scripts/nofollow.py @@ -53,7 +53,8 @@ def open_below(root: Path, path: Path, flags: int, mode: int) -> int: mode: permission bits of a created file. Returns: - The file descriptor of a regular file; the caller closes it. + The file descriptor of a regular file with a single link; the caller + closes it. """ *dirs, name = parts_below(root, path) fd = directory(root, tuple(dirs), bool(flags & os.O_CREAT)) @@ -61,9 +62,10 @@ def open_below(root: Path, path: Path, flags: int, mode: int) -> int: file = os.open(name, flags | os.O_NOFOLLOW | os.O_NONBLOCK | os.O_CLOEXEC, mode, dir_fd=fd) finally: os.close(fd) - if not stat.S_ISREG(os.fstat(file).st_mode): + info = os.fstat(file) + if not stat.S_ISREG(info.st_mode) or info.st_nlink != 1: os.close(file) - raise OSError(errno.EINVAL, "not a regular file", str(path)) + raise OSError(errno.EINVAL, "not a regular file with a single link", str(path)) return file diff --git a/skills/kvb-org-rasci/scripts/rasci.py b/skills/kvb-org-rasci/scripts/rasci.py index c15d898..1834347 100755 --- a/skills/kvb-org-rasci/scripts/rasci.py +++ b/skills/kvb-org-rasci/scripts/rasci.py @@ -15,21 +15,22 @@ from __future__ import annotations import argparse import contextlib import datetime -import fcntl import importlib.util import json import os import re import statistics import sys -import tempfile from collections.abc import Iterator from pathlib import Path +from types import ModuleType ROOT = Path(os.environ.get("AGENTS_COUNCIL_ROOT", "/tmp/agents-council")) DB = ROOT / "rasci.json" LOCK = ROOT / "rasci.lock" -COUNCIL = Path(__file__).resolve().parents[2] / "kvb-org-organize" / "scripts" / "council.py" +FILE_MODE = 0o600 +ORGANIZE = Path(__file__).resolve().parents[2] / "kvb-org-organize" / "scripts" +COUNCIL = ORGANIZE / "council.py" TASK = re.compile(r"^[a-z0-9][a-z0-9._-]*(/[a-z0-9][a-z0-9._-]*)*$") LETTERS = re.compile(r"^(?!.*(.).*\1)[RASCI]+$") ORDER = "RASCI" @@ -75,27 +76,41 @@ def letters_of(*parts: str) -> str: return "".join(letter for letter in ORDER if any(letter in part for part in parts)) +def script(name: str, path: Path) -> ModuleType: + spec = importlib.util.spec_from_file_location(name, path) + loaded = importlib.util.module_from_spec(spec) + spec.loader.exec_module(loaded) + return loaded + + +nofollow = script("kvb_rasci_nofollow", ORGANIZE / "nofollow.py") + + def load() -> dict[str, dict]: try: - return json.loads(DB.read_text(encoding="utf-8")) + tasks = json.loads(nofollow.read_text(ROOT, DB)) except FileNotFoundError: return {} - except ValueError: + except (ValueError, RecursionError): fail(f"{DB} is not valid JSON") + except OSError as error: + fail(f"{DB} is no regular file below {ROOT}: {error.strerror}") + if not isinstance(tasks, dict) or not all(isinstance(entry, dict) for entry in tasks.values()): + fail(f"{DB} is not an object of task objects") + return tasks @contextlib.contextmanager def database() -> Iterator[dict[str, dict]]: - ROOT.mkdir(parents=True, exist_ok=True) - with LOCK.open("a") as lock: - fcntl.flock(lock, fcntl.LOCK_EX) + with contextlib.ExitStack() as held: + try: + ROOT.mkdir(parents=True, exist_ok=True) + held.enter_context(nofollow.locked(ROOT, LOCK, FILE_MODE, None)) + except OSError as error: + fail(f"cannot lock {LOCK}: {error.strerror}") tasks = load() yield tasks - with tempfile.NamedTemporaryFile( - "w", dir=ROOT, prefix=".rasci-", delete=False, encoding="utf-8" - ) as tmp: - json.dump(tasks, tmp, indent=1, sort_keys=True) - os.replace(tmp.name, DB) + nofollow.replace(ROOT, DB, json.dumps(tasks, indent=1, sort_keys=True), FILE_MODE) def open_results(entry: dict) -> int: @@ -152,9 +167,7 @@ def track(task: str, holder: str, status: str, *, description: str | None = None def claimed() -> dict[str, dict]: if not COUNCIL.is_file(): return {} - spec = importlib.util.spec_from_file_location("council", COUNCIL) - council = importlib.util.module_from_spec(spec) - spec.loader.exec_module(council) + council = script("council", COUNCIL) live = council.live_sessions() rows = {} for office, claim in council.claims(): diff --git a/tests/test_nofollow.py b/tests/test_nofollow.py index 933af24..f158194 100644 --- a/tests/test_nofollow.py +++ b/tests/test_nofollow.py @@ -100,6 +100,17 @@ class TestNofollow(unittest.TestCase): write(link) self.assertFalse(dangling.exists()) + def test_a_hard_linked_name_is_refused_and_its_target_kept(self): + followers = {name: write for name, write in self.writes(self.root).items() if name not in ("replace", "create")} + for name, write in followers.items(): + with self.subTest(write=name): + link = self.root / name + os.link(self.victim, link) + with self.assertRaises(OSError): + write(link) + self.assertEqual(self.victim.read_text(encoding="utf-8"), "keep") + link.unlink() + def test_a_symlinked_directory_is_refused_and_nothing_lands_in_its_target(self): (self.root / "d").symlink_to(self.victims) (self.victims / "f").write_text("keep", encoding="utf-8") diff --git a/tests/test_rasci.py b/tests/test_rasci.py index 2223851..90b2bc3 100644 --- a/tests/test_rasci.py +++ b/tests/test_rasci.py @@ -4,6 +4,7 @@ from __future__ import annotations import datetime import json +import os import subprocess import sys import tempfile @@ -35,7 +36,7 @@ class TestRasci(unittest.TestCase): json.dumps({"sessionId": sid, "name": name}), encoding="utf-8" ) - def _run(self, *args: str, script: Path = RASCI, sid: str = "s1") -> subprocess.CompletedProcess: + def _run(self, *args: str, script: Path = RASCI, sid: str = "s1", timeout: float | None = None) -> subprocess.CompletedProcess: env = { "PATH": "/usr/bin:/bin", "HOME": str(self.home), @@ -49,6 +50,7 @@ class TestRasci(unittest.TestCase): env=env, cwd=self.tmp.name, check=False, + timeout=timeout, ) def _out(self, *args: str) -> list[str]: @@ -333,6 +335,56 @@ class TestRasci(unittest.TestCase): with self.subTest(args=args): self.assertEqual(self._run(*args).returncode, 2) + def test_a_symlinked_register_is_refused_and_never_copied_into_the_council(self): + outside = Path(self.tmp.name) / "outside.json" + outside.write_text(json.dumps({"outside": {"title": "marker-outside"}}), encoding="utf-8") + self.root.mkdir() + (self.root / "rasci.json").symlink_to(outside) + for args in (("show", "--all"), ("set", "work/a", "--title", "A")): + with self.subTest(args=args): + result = self._run(*args, timeout=60) + self.assertEqual(result.returncode, 2, result.stderr) + self.assertNotIn("outside", result.stdout) + self.assertTrue((self.root / "rasci.json").is_symlink()) + copies = [p.name for p in self.root.iterdir() if p.is_file() and not p.is_symlink() + and "marker-outside" in p.read_text(encoding="utf-8")] + self.assertEqual(copies, []) + + def test_a_hard_linked_register_is_refused(self): + outside = Path(self.tmp.name) / "outside.json" + outside.write_text(json.dumps({"outside": {"title": "marker-outside"}}), encoding="utf-8") + self.root.mkdir() + os.link(outside, self.root / "rasci.json") + result = self._run("show", "--all", timeout=60) + self.assertEqual(result.returncode, 2, result.stderr) + self.assertNotIn("outside", result.stdout) + + def test_a_symlinked_lock_is_refused_and_creates_nothing_at_its_target(self): + aimed = Path(self.tmp.name) / "elsewhere" / "aimed" + aimed.parent.mkdir() + self.root.mkdir() + (self.root / "rasci.lock").symlink_to(aimed) + result = self._run("set", "work/a", "--title", "A", timeout=60) + self.assertEqual(result.returncode, 2, result.stderr) + self.assertFalse(aimed.exists()) + self.assertFalse((self.root / "rasci.json").exists()) + + def test_a_fifo_register_fails_instead_of_hanging(self): + self.root.mkdir() + os.mkfifo(self.root / "rasci.json") + for args in (("show",), ("set", "work/a")): + with self.subTest(args=args): + self.assertEqual(self._run(*args, timeout=20).returncode, 2) + + def test_a_register_that_is_no_object_of_tasks_exits_2(self): + self.root.mkdir() + for text in ("[1, 2]", json.dumps({"work/a": 3}), "[" * 100000): + with self.subTest(text=text[:12]): + (self.root / "rasci.json").write_text(text, encoding="utf-8") + result = self._run("show", "--all", timeout=60) + self.assertEqual(result.returncode, 2, result.stderr) + self.assertIn("rasci:", result.stderr) + class TestOrgSkillsUseTheRegister(unittest.TestCase): def test_every_org_skill_names_the_register(self):