fix(kvb-org-rasci): open the register and its lock without following links
The council root is writable by every agent, and operator.py runs rasci.database() outside the sandbox. A symlinked rasci.json made it read any JSON file the operator can read and write it back into the council as a regular file; a symlinked rasci.lock made it create files at the link target; a FIFO hung every register call. load() now reads through nofollow.read_text and database() locks and writes through nofollow.locked and nofollow.replace. A register that is no object of task objects, or nested past the recursion limit, exits 2 instead of a traceback. nofollow.open_below also refuses a file with more than one link, so a hard link to a file outside the council is refused like a symlink. 🎯 Fix: 96 % 💥 Break: 3 % 💸 Cost: 99 % ⏱️ Time: 95 % 🛡️ Security: 100 % 🏆 Score: 99 🚦 Gate: ✅ 90/5, cost and time <= 100, security 100 % Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
13b360227b
commit
8e180be256
4 files changed
+98
-20
No files matched your search
@@ -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
|
||||
|
||||
|
||||
|
||||
@@ -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():
|
||||
|
||||
@@ -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")
|
||||
|
||||
+53
-1
@@ -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):
|
||||
|
||||
Reference in new issue
Block a user