From 8a8c3634e8d64389000903ffd137f0d29665ecb3 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Mon, 14 Sep 2026 22:46:06 -0700 Subject: [PATCH] fix(kanban): scope the delegated-child write fence to the lineage's board root HERMES_DELEGATED_CHILD_CONTEXT=1 is deliberately carried into every shell/ execute_code subprocess a delegate_task child spawns (the fence must survive exec so a grandchild `hermes kanban complete` cannot promote itself). But the readers treated the bare flag as "fence every Kanban DB": kanban_db_connect opened ANY board ?mode=ro and write_txn refused ANY mutation. A subagent running a Kanban reproduction against a scratch HERMES_HOME therefore got a silently read-only board with a misleading "descendants require an initialized board" error; only one lane in the retrospective ever discovered why (deleg_15dac332), every earlier kanban repro ran degraded. The marker's value is now the fenced board ROOT (kanban_home() at spawn) and readers deny only paths under that root or the dispatcher-pinned HERMES_KANBAN_DB (kanban_path_is_fenced). In-process children and a legacy "1" marker still fence everything; an inherited path marker is never re-derived, so a grandchild that moved HERMES_HOME cannot unfence the real board. Owner-gate tests (test_kanban_descendant_scope, cron env isolation, kanban CLI exit status) are unchanged and green. --- agent/delegation_context.py | 43 +++++++++++++++- hermes_cli/kanban.py | 7 +-- hermes_cli/kanban_db.py | 12 ++--- hermes_cli/kanban_db_connect.py | 17 +++++-- tests/tools/test_delegate_kanban_isolation.py | 2 +- tests/tools/test_hermes_subprocess_env.py | 2 +- tests/tools/test_kanban_descendant_scope.py | 49 +++++++++++++++++++ .../features/kanban-worker-lanes.md | 3 ++ 8 files changed, 117 insertions(+), 18 deletions(-) diff --git a/agent/delegation_context.py b/agent/delegation_context.py index 959c9df85b..4ea7f895f0 100644 --- a/agent/delegation_context.py +++ b/agent/delegation_context.py @@ -78,18 +78,59 @@ def is_delegated_child_process_context() -> bool: return bool(_DELEGATED_CHILD_CONTEXT.get()) or bool(os.environ.get(DELEGATED_CHILD_ENV_MARKER)) +def _fenced_kanban_root() -> str: + """The board root this process's Kanban lineage lives under (``kanban_home()``); ``"1"`` when it + cannot be resolved, which readers treat as "fence every board" (the pre-path marker).""" + try: + from hermes_cli.kanban_db import kanban_home + return str(kanban_home()) + except Exception: + return "1" + + def scrub_kanban_env(env: Mapping[str, str] | MutableMapping[str, str]) -> dict[str, str]: """Remove worker identity, retaining board/location and an inherited write fence. TASK absence alone would promote a descendant to an orchestrator. The marker survives later execs, including scripts that remove TASK themselves. This is cooperative runtime scoping, not confinement of code with direct SQLite access. + + The marker's value is the fenced board ROOT, so the fence applies to the lineage's + board and not to every Kanban DB the descendant touches: a child running a repro + against a temp ``HERMES_HOME`` got a silently read-only board there. An inherited + path-valued marker is kept (a grandchild that moved HERMES_HOME must not re-fence + onto its scratch root and unfence the real one). """ cleaned = {k: v for k, v in env.items() if k not in KANBAN_ENV_KEYS} - cleaned[DELEGATED_CHILD_ENV_MARKER] = "1" + inherited = str(env.get(DELEGATED_CHILD_ENV_MARKER) or "") + cleaned[DELEGATED_CHILD_ENV_MARKER] = inherited if inherited and inherited != "1" else _fenced_kanban_root() return cleaned +def kanban_path_is_fenced(path: "os.PathLike[str] | str") -> bool: + """Whether Kanban mutations at *path* (a board DB or board-metadata root) are denied for this + process: always for an in-process delegate child (the parent's own board); for a spawned + descendant only when *path* is the dispatcher-pinned ``HERMES_KANBAN_DB`` or lies under the + fenced root the marker carries. A legacy ``"1"`` marker fences everything.""" + if _DELEGATED_CHILD_CONTEXT.get(): + return True + marker = os.environ.get(DELEGATED_CHILD_ENV_MARKER, "") + if not marker: + return False + if marker == "1": + return True + from pathlib import Path + target = Path(path).expanduser().resolve() + pinned = os.environ.get("HERMES_KANBAN_DB", "").strip() + if pinned and target == Path(pinned).expanduser().resolve(): + return True + try: + target.relative_to(Path(marker).expanduser().resolve()) + except ValueError: + return False + return True + + @overload def delegated_child_subprocess_env(env: Mapping[str, str]) -> dict[str, str]: ... diff --git a/hermes_cli/kanban.py b/hermes_cli/kanban.py index a9f66b9252..204fbdeecf 100644 --- a/hermes_cli/kanban.py +++ b/hermes_cli/kanban.py @@ -233,12 +233,9 @@ def _is_delegated_child_cli_mutation(args: argparse.Namespace) -> bool: return False elif action not in _DELEGATED_CHILD_DENIED_ACTIONS: return False - try: - from agent.delegation_context import is_delegated_child_process_context + from agent.delegation_context import kanban_path_is_fenced - return is_delegated_child_process_context() - except Exception: - return bool(os.environ.get("HERMES_DELEGATED_CHILD_CONTEXT")) + return kanban_path_is_fenced(kb.kanban_home()) or kanban_path_is_fenced(kb.kanban_db_path()) def _joined_words(words) -> Optional[str]: diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index 91d6dc724f..53a22a0e06 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -118,20 +118,18 @@ _IS_WINDOWS = sys.platform == "win32" KANBAN_ATTACHMENT_MAX_BYTES = 25 * 1024 * 1024 # one cap for dashboard, tools and CLI -def _assert_not_delegated_child_mutation() -> None: +def _assert_not_delegated_child_mutation(path: "str | Path | None" = None) -> None: """Reject Kanban mutations from ``delegate_task`` child contexts. The tool/CLI fast-fail guards are UX, not a trust boundary (a child can shell out or import this module); the invariant lives here so every ``write_txn`` user and board-metadata mutator fails closed before touching durable state. + *path* is the board DB / metadata root being mutated; ``None`` means the + lineage's own board (``kanban_home()``). """ - try: - from agent.delegation_context import is_delegated_child_process_context + from agent.delegation_context import kanban_path_is_fenced - delegated = is_delegated_child_process_context() - except Exception: - delegated = bool(os.environ.get("HERMES_DELEGATED_CHILD_CONTEXT")) - if delegated: + if kanban_path_is_fenced(kanban_home() if path is None else path): raise PermissionError("delegate_task child contexts cannot mutate Kanban tasks or boards") diff --git a/hermes_cli/kanban_db_connect.py b/hermes_cli/kanban_db_connect.py index e6debc811b..9b9eaad49d 100644 --- a/hermes_cli/kanban_db_connect.py +++ b/hermes_cli/kanban_db_connect.py @@ -672,8 +672,8 @@ def connect(db_path: Optional[Path] = None, *, board: Optional[str] = None) -> s :func:`kanban_db_path` (``HERMES_KANBAN_DB`` -> ``HERMES_KANBAN_BOARD`` -> ``/kanban/current`` -> ``default``).""" path = db_path if db_path is not None else _kb.kanban_db_path(board=board) - from agent.delegation_context import is_delegated_child_process_context - if is_delegated_child_process_context(): + from agent.delegation_context import kanban_path_is_fenced + if kanban_path_is_fenced(path): # Reads must not enter schema/backfill write transactions. Never create a # missing board or migrate on a descendant's behalf; the owner initializes it. conn = sqlite3.connect(path.resolve().as_uri() + "?mode=ro", uri=True) @@ -1141,6 +1141,17 @@ def _execute_boundary_with_retry(conn: sqlite3.Connection, sql: str) -> None: time.sleep(random.uniform(_BUSY_RETRY_MIN_S, _BUSY_RETRY_MAX_S)) +def _main_db_file(conn: sqlite3.Connection) -> Optional[str]: + """Filesystem path of *conn*'s main database (None for in-memory / unreadable).""" + try: + for _seq, name, file in conn.execute("PRAGMA database_list") or (): + if name == "main": + return file or None + except (sqlite3.Error, TypeError, ValueError): + pass + return None + + @contextlib.contextmanager def write_txn(conn: sqlite3.Connection, *, allow_nested: bool = False): """IMMEDIATE write transaction; a claim CAS inside is atomic — at most one @@ -1152,7 +1163,7 @@ def write_txn(conn: sqlite3.Connection, *, allow_nested: bool = False): (``complete_task`` & co.) must never run under an open outer transaction, since those side effects would fire while the outer txn can still roll back. """ - _kb._assert_not_delegated_child_mutation() + _kb._assert_not_delegated_child_mutation(_main_db_file(conn)) if getattr(conn, "in_transaction", False): if not allow_nested: raise RuntimeError( diff --git a/tests/tools/test_delegate_kanban_isolation.py b/tests/tools/test_delegate_kanban_isolation.py index a248590c18..9d053d0570 100644 --- a/tests/tools/test_delegate_kanban_isolation.py +++ b/tests/tools/test_delegate_kanban_isolation.py @@ -165,7 +165,7 @@ def test_delegate_child_execute_code_env_bridges_contextvar_and_scrubs_kanban( assert os.environ.get("HERMES_DELEGATED_CHILD_CONTEXT") is None assert env["HERMES_HOME"] == str(home) - assert env["HERMES_DELEGATED_CHILD_CONTEXT"] == "1" + assert env["HERMES_DELEGATED_CHILD_CONTEXT"] # fenced board root (path), not a bare flag assert "HERMES_KANBAN_TASK" not in env assert "HERMES_KANBAN_RUN_ID" not in env assert "HERMES_KANBAN_CLAIM_LOCK" not in env diff --git a/tests/tools/test_hermes_subprocess_env.py b/tests/tools/test_hermes_subprocess_env.py index bac2c5d5e7..b3381a50cb 100644 --- a/tests/tools/test_hermes_subprocess_env.py +++ b/tests/tools/test_hermes_subprocess_env.py @@ -168,7 +168,7 @@ class TestDelegatedChildMarker: with delegated_child_context(): env = hermes_subprocess_env(inherit_credentials=True) - assert env["HERMES_DELEGATED_CHILD_CONTEXT"] == "1" + assert env["HERMES_DELEGATED_CHILD_CONTEXT"] # fenced board root (path), not a bare flag # Worker identity is scrubbed; board location and workspace routing survive so the # fenced descendant can still read the board it belongs to. assert "HERMES_KANBAN_TASK" not in env diff --git a/tests/tools/test_kanban_descendant_scope.py b/tests/tools/test_kanban_descendant_scope.py index a78f658281..d10f32ff9c 100644 --- a/tests/tools/test_kanban_descendant_scope.py +++ b/tests/tools/test_kanban_descendant_scope.py @@ -107,3 +107,52 @@ def test_worker_cli_cannot_use_foreign_task_to_drop_run_scope(tmp_path, monkeypa assert not kb.list_attachments(conn, foreign) assert json.loads(kanban_tools._handle_complete({"task_id": own, "summary": "parent"}))["ok"] conn.close() + + +def test_child_shell_can_write_a_kanban_board_outside_its_lineage_root(tmp_path, monkeypatch): + """The fence a delegate_task child inherits applies to ITS lineage's board, not to every Kanban + DB its shell touches: a repro run against a scratch HERMES_HOME got a silently read-only board + (``connect`` opened ``?mode=ro``; ``write_txn`` raised PermissionError). Real ``terminal`` + ingress, real subprocess, real SQLite — the lineage board stays fenced in the same shell.""" + from agent.delegation_context import delegated_child_context + + lineage_home = tmp_path / "lineage" + scratch_home = tmp_path / "scratch" + for home in (lineage_home, scratch_home): + home.mkdir() + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.setenv("HERMES_HOME", str(lineage_home)) + monkeypatch.delenv("HERMES_DELEGATED_CHILD_CONTEXT", raising=False) + for key in ("HERMES_KANBAN_DB", "HERMES_KANBAN_BOARD", "HERMES_KANBAN_TASK", "HERMES_KANBAN_HOME"): + monkeypatch.delenv(key, raising=False) + connect(kb.kanban_db_path()).close() # the owner initializes the lineage board + script = tmp_path / "repro.py" + script.write_text( + "import os, sys, json\n" + f"sys.path.insert(0, {str(ROOT)!r})\n" + "from hermes_cli import kanban_db as kb\n" + "from hermes_cli.kanban_db_connect import connect\n" + "out = {'marker': os.environ.get('HERMES_DELEGATED_CHILD_CONTEXT')}\n" + "try:\n" + " conn = connect(kb.kanban_db_path()); kb.create_task(conn, title='lineage'); out['lineage'] = 'WROTE'\n" + "except PermissionError as exc:\n" + " out['lineage'] = 'fenced: ' + str(exc)\n" + f"os.environ['HERMES_HOME'] = {str(scratch_home)!r}\n" + "import hermes_constants; hermes_constants._default_hermes_root_memo = None\n" + "conn = connect(kb.kanban_db_path()); out['scratch'] = kb.create_task(conn, title='scratch')\n" + "print('SCOPE_RESULT=' + json.dumps(out))\n" + ) + with delegated_child_context("child-repro"): + terminal = LocalEnvironment(cwd=str(tmp_path)) + try: + result = terminal.execute(f"{shlex.quote(sys.executable)} {shlex.quote(str(script))}") + finally: + terminal.cleanup() + output = result.get("output", "") + row = json.loads(next(line.split("SCOPE_RESULT=", 1)[1] for line in output.splitlines() if "SCOPE_RESULT=" in line)) + assert row["marker"] and row["marker"] != "1", row + assert row["lineage"].startswith("fenced"), row + assert row["scratch"], row + scratch_conn = connect(scratch_home / "kanban.db") + assert kb.get_task(scratch_conn, row["scratch"]).title == "scratch" + scratch_conn.close() diff --git a/website/docs/user-guide/features/kanban-worker-lanes.md b/website/docs/user-guide/features/kanban-worker-lanes.md index 7a7141cea3..39c7d53c2b 100644 --- a/website/docs/user-guide/features/kanban-worker-lanes.md +++ b/website/docs/user-guide/features/kanban-worker-lanes.md @@ -55,6 +55,9 @@ children remain fenced even when a script removes the inherited task ID: CLI and tool mutations are rejected, rather than treating that script as an orchestrator. Board/database routing and workspace paths are retained. Descendants can read an existing board without running schema migrations; its owner must initialize it. +The fence is scoped to the lineage's board root (the marker's value is that root, plus the +dispatcher-pinned `HERMES_KANBAN_DB`): a descendant that works against a different Kanban +home — a test or reproduction under a scratch `HERMES_HOME` — gets a normal read-write board. The dispatcher explicitly grants a newly assigned worker its own scope. The managed Hermes-tools MCP endpoint can likewise act for its supervising worker, while the