emrg: a session id is one path component, checked where every message crosses

emrg/server/daemon.py

@@ -93,7 +93,12 @@
     ServerPong,
     TaskRequest,
 )
-from emrg.session import Session, last_n_messages, records_to_messages
+from emrg.session import (
+    Session,
+    is_safe_session_id,
+    last_n_messages,
+    records_to_messages,
+)
 
 
 # ── 会话级 sandbox 档位（rant 2026-09-30T09:30:16）─────────────────────
@@ -1570,6 +1575,12 @@ async def _handle_client(self, ws) -> None:
                             "error": "task requires session_id and cwd",
                         })
                         continue
+                    # This branch is the one door `_process_message` does not own, and it
+                    # is the door that creates the session directory: without this call a
+                    # `task` carrying an unsafe id reached `_get_or_create_session` and
+                    # wrote outside `<cwd>/.emrg/sessions` (measured on this branch).
+                    if await self._refuse_an_unsafe_session_id(ws, "task", session_id):
+                        continue
                     # Phase 2 session-level lock (protocol-contract §2.6.5):
                     # one active task per session — concurrent tasks queue.
                     # P1 (rant 21:55:37): construct req + allow_tools FIRST
@@ -2783,12 +2794,52 @@ def _collect_history_data(self, session: Session) -> dict[str, str]:
             "date_str": datetime.now().strftime("%y%m%d"),
         }
 
+    async def _refuse_an_unsafe_session_id(
+        self, ws, msg_type: str, session_id: object
+    ) -> bool:
+        """Refuse a client-supplied session id that is not one path component.
+
+        True means the id was refused and the client has been told, so the caller stops.
+
+        A session id arrives in a client message and lands on a path in half the handlers
+        below: `_get_or_create_session` (eleven callers) writes it, the resume reader and
+        the two memory readers open it, and `delete_session` hands the resolved path to
+        `shutil.rmtree`. Measured 2026-10-21: `Session.delete("../..", cwd)` walked
+        `cwd/.emrg/sessions/../..` and left the project tree empty.
+
+        **Two dispatchers, not one.** `_handle_client` intercepts `{"type": "task"}`
+        itself and never reaches `_process_message`, so a check placed only in the latter
+        left the one message type that *creates* a session directory unguarded — measured
+        on this branch before the fix: a `task` carrying `../../../evil` created a
+        directory one level above the project root and started its turn, telling the
+        client nothing. Both call sites ask this method, so the rule has one writer even
+        though it has two doors.
+        """
+        if not session_id or is_safe_session_id(session_id):
+            return False
+        logger.warning(
+            "refused a %r message carrying session id %r: a session id is one "
+            "path component inside <cwd>/.emrg/sessions", msg_type, session_id,
+        )
+        await self._send(ws, {
+            "error": f"invalid session id: {session_id!r} (a session id is "
+                     f"one path component)",
+        })
+        return True
+
     async def _process_message(
         self, msg: dict, ws
     ) -> None:
         """Process a single message and send responses."""
         msg_type = msg.get("type", "")
 
+        # Every message type dispatched below takes its session id from this message and
+        # most of them paste it onto `<cwd>/.emrg/sessions/`. Checked here, before any
+        # handler builds a path; `{"type": "task"}` is checked in `_handle_client`,
+        # which never reaches this method.
+        if await self._refuse_an_unsafe_session_id(ws, msg_type, msg.get("session_id")):
+            return
+
         if msg_type == "ping":
             elapsed = int(
                 (datetime.now() - self.start_time).total_seconds()

emrg/server/prompts/system.j2

@@ -37,7 +37,7 @@ Each skill is a file of instructions. When one bears on what the host asked, rea
 
 - **Every call carries an `intent`**: one plain sentence on why it is made and what it should reach. It is never executed, but the host reads it, so it must be true and specific.
 - **Read a file before you change it**, so the change matches what is there.
-- **Issue every call you can already specify, in one response.** Sending several is the normal case: a call goes alone only when its arguments need an answer you have not read yet, and then its `intent` names the specific answer it waits for. Every round re-sends the conversation and emits fresh output.
+- **Plan the round before the first call.** Enumerate what this round has to establish and the readings it needs, and write that enumeration out as the opening lines of your reply, before you call anything. Every reading you can already specify goes out in one response; a call stands alone only when its arguments need an answer you have not read yet. The plan is revised between rounds, when a reading changes what the round can still settle — never between two calls of the same round. One round re-sends the whole conversation and emits fresh output, so a lone call buys a single reading at the price of a round.
 
 ## Memory
 

emrg/session.py

@@ -16,6 +16,7 @@
 import hashlib
 import json
 import logging
+import ntpath
 import os
 import secrets
 from datetime import datetime
@@ -306,6 +307,39 @@ def _refuse_a_non_regular_subject(path: Path) -> None:
         )
 
 
+#: Characters that make a name more than one path component, on either platform named
+#: here: `/` separates on POSIX and Windows, `\` separates on Windows and is an ordinary
+#: character on POSIX — refusing both is the point, because the same id reaches a path on
+#: whichever platform runs it.
+_NAME_SEPARATORS = ("/", "\\")
+
+
+def is_safe_session_id(session_id: object) -> bool:
+    """True when `session_id` names exactly one entry inside the sessions directory.
+
+    A session id arrives in a client message, so it is an input like any other:
+    `Path(cwd) / ".emrg" / "sessions" / session_id` hands `..` and an absolute id to the
+    kernel, which resolves them, and the caller then removes what it resolved to. Measured
+    2026-10-21: `Session.delete("../..", cwd)` walked `cwd/.emrg/sessions/../..` — the
+    project tree — and left it empty, `victim/` and `.emrg/` alike, then raised from
+    `shutil.rmtree`'s final `os.rmdir`.
+
+    So the id is checked before a path is built: non-empty, no NUL, not `.` or `..`, no
+    separator, and no drive. The drive is asked of **both** flavours — `os.path.splitdrive`
+    answers `("", "C:evil")` on this host and `ntpath.splitdrive` answers `("C:", "evil")`,
+    and a `C:`-relative id is a path, not a name, on the platform that can receive it.
+    """
+    if not isinstance(session_id, str) or not session_id:
+        return False
+    if "\0" in session_id or session_id in (".", ".."):
+        return False
+    if any(sep in session_id for sep in _NAME_SEPARATORS):
+        return False
+    if os.path.splitdrive(session_id)[0] or ntpath.splitdrive(session_id)[0]:
+        return False
+    return True
+
+
 class Session:
     """Manages a single conversation session on disk."""
 
@@ -981,10 +1015,20 @@ def clear(self) -> None:
     def delete(session_id: str, cwd: Path) -> bool:
         """Delete a session directory and all its contents.
 
-        Returns True if the session was deleted, False if it didn't exist.
+        Returns True if the session was deleted, False if it didn't exist — and False,
+        with a warning, for an id that is not a single path component, because the id
+        comes from a client message and `shutil.rmtree` follows whatever it resolves to.
         """
         import shutil
 
+        if not is_safe_session_id(session_id):
+            logger.warning(
+                "refusing to delete a session named %r: a session id is one path "
+                "component, so nothing outside the sessions directory is reachable",
+                session_id,
+            )
+            return False
+
         session_dir = cwd / ".emrg" / "sessions" / session_id
         if session_dir.exists():
             shutil.rmtree(session_dir)

tests/test_daemon.py

@@ -5622,3 +5622,143 @@ def test_an_out_of_domain_limit_is_refused_rather_than_coerced(tmp_path):
 
     assert frames[0]["type"] == "input_history"
     assert "integer limit and offset" in frames[0]["error"]
+
+
+# ── a session id is one path component, at both doors a message can use ────────
+#
+# The id is pasted onto `<cwd>/.emrg/sessions/` by eleven `_get_or_create_session`
+# call sites, by the resume reader, by the two memory readers and by
+# `delete_session`, which hands the resolved path to `shutil.rmtree`. So the check
+# lives once, in `_refuse_an_unsafe_session_id`: measured 2026-10-21, an id of
+# `../..` reached `rmtree` with the project tree.
+#
+# It has **two** callers because the daemon has two dispatchers. `_handle_client`
+# intercepts `{"type": "task"}` itself and never reaches `_process_message`, so a
+# check placed only in the latter left unguarded the one message type that creates
+# a session directory — measured on this branch before the fix: a `task` carrying
+# `../../../evil` created a directory one level above the project root, started its
+# turn, and sent the client nothing but `auth_ok`. A test that hands a `task` frame
+# to `_process_message` therefore certifies a door production does not use: that
+# function has no `task` branch at all and answers `unknown message type`. The two
+# tests below drive `_handle_client`, which is the door.
+
+
+def _project_with_a_neighbour(tmp_path):
+    (tmp_path / ".emrg" / "sessions" / "keep").mkdir(parents=True)
+    (tmp_path / "victim").mkdir()
+    (tmp_path / "victim" / "precious.txt").write_text("keep me", encoding="utf-8")
+    return tmp_path
+
+
+class _FakeSocket:
+    """A websocket stand-in the read loop can drive: `recv` yields frames, then ends."""
+
+    def __init__(self, frames: list[dict]) -> None:
+        self._frames = [json.dumps(f) for f in frames]
+        self.sent: list[dict] = []
+
+    async def recv(self) -> str:
+        if not self._frames:
+            from websockets.exceptions import ConnectionClosed
+            raise ConnectionClosed(None, None)
+        return self._frames.pop(0)
+
+    async def send(self, data) -> None:
+        self.sent.append(json.loads(data))
+
+    async def close(self) -> None:
+        pass
+
+
+def _drive_read_loop(monkeypatch, tmp_path, frames: list[dict]) -> list[dict]:
+    """One real `_handle_client` connection: auth, then `frames`, then a disconnect.
+
+    `_auth_token` is set by `EmrgServer.run()` when it binds a socket, and a unit
+    test never binds one, so it is pinned here. The two planted-fire markers are
+    pointed at `tmp_path` because a `task` frame really does start a tool loop, and
+    the loop stamps host state at round finalization (issue #1337).
+    """
+    monkeypatch.setattr(daemon_mod, "_PLANTED_FIRE_MARKER_PATH", tmp_path / "planted-fire")
+    monkeypatch.setattr(daemon_mod, "_PLANTED_FIRE_ROUND_COMPLETE_PATH",
+                        tmp_path / "planted-fire-round")
+    server = _make_server()
+    server._auth_token = "test-token"
+    socket = _FakeSocket([{"type": "auth", "token": "test-token"}, *frames])
+    asyncio.run(server._handle_client(socket))
+    return socket.sent
+
+
+def test_delete_session_refuses_an_id_that_is_not_one_component(tmp_path):
+    """A refused delete leaves the project alone and says so in a frame."""
+    project = _project_with_a_neighbour(tmp_path)
+    server = _make_server()
+
+    frames = _drive(server, {
+        "type": "delete_session", "session_id": "../..", "cwd": str(project),
+    })
+
+    assert len(frames) == 1
+    assert "invalid session id" in frames[0]["error"]
+    assert (project / "victim" / "precious.txt").read_text(encoding="utf-8") == "keep me"
+    assert (project / ".emrg" / "sessions" / "keep").is_dir()
+
+
+def test_delete_session_still_deletes_a_real_one(tmp_path):
+    """The other direction on the same seam: a plain id is untouched by the refusal."""
+    project = _project_with_a_neighbour(tmp_path)
+    server = _make_server()
+
+    frames = _drive(server, {
+        "type": "delete_session", "session_id": "keep", "cwd": str(project),
+    })
+
+    assert frames[0]["type"] == "session_deleted"
+    assert frames[0].get("error") is None
+    assert not (project / ".emrg" / "sessions" / "keep").exists()
+    assert (project / "victim" / "precious.txt").exists()
+
+
+def test_the_read_loop_refuses_a_task_carrying_an_unsafe_session_id(
+    tmp_path, monkeypatch
+):
+    """`_get_or_create_session` writes the directory, so the refusal precedes it.
+
+    Driven through `_handle_client`, because that is where a `task` is dispatched.
+    The id walks two levels up from the sessions directory, so without the refusal
+    the directory it creates lands **outside** the project — which is what the
+    assertion on `tmp_path` measures, not on `sessions/`.
+    """
+    project = tmp_path / "project"
+    (project / ".emrg" / "sessions" / "keep").mkdir(parents=True)
+
+    frames = _drive_read_loop(monkeypatch, tmp_path, [{
+        "type": "task", "id": "t1", "session_id": "../../../evil",
+        "cwd": str(project), "prompt": "hi",
+    }])
+
+    assert [f for f in frames if f.get("type") != "auth_ok"] == [
+        {"error": "invalid session id: '../../../evil' (a session id is one path component)"}
+    ]
+    assert not (tmp_path / "evil").exists()
+    assert sorted(p.name for p in (project / ".emrg" / "sessions").iterdir()) == ["keep"]
+
+
+def test_the_read_loop_still_accepts_a_task_carrying_a_plain_session_id(
+    tmp_path, monkeypatch
+):
+    """The other direction at the same door: a plain id creates its own session and starts.
+
+    The turn's LLM call cannot be reached in a unit test, so the assertion is the one
+    that discriminates: no `invalid session id` frame, and the session directory the
+    branch creates exists.
+    """
+    project = tmp_path / "project"
+    (project / ".emrg" / "sessions" / "keep").mkdir(parents=True)
+
+    frames = _drive_read_loop(monkeypatch, tmp_path, [{
+        "type": "task", "id": "t2", "session_id": "fresh",
+        "cwd": str(project), "prompt": "hi",
+    }])
+
+    assert not [f for f in frames if "invalid session id" in str(f.get("error", ""))]
+    assert (project / ".emrg" / "sessions" / "fresh").is_dir()

tests/test_evolution_prompt_round_cost.py

@@ -72,21 +72,28 @@
     ),
 }
 
-#: The rule's unit, its criterion and its boundary, as shipped in **both** carriers. Editing either
+#: The rule's unit, its criterion and its reason, as shipped in **both** carriers. Editing either
 #: one alone is the defect this pair catches, so `test_both_carriers_state_the_same_unit_and_criterion`
 #: reads the live text of each rather than one file's expectations about the other.
 #:
 #: These are the longest phrases the two carriers genuinely share, and that is a measurement rather
 #: than a preference: R10 is a rulebook entry a cycle reads and `system.j2` is one bullet in every
-#: session's prefix, so the two state the same rule at different lengths — `issue every call you can
-#: already specify, in one response: sending several tool calls in one round … is the normal case`
-#: against `**Issue every call you can already specify, in one response.** Sending several is the
-#: normal case: …`. The phrases below are where they agree word for word (the rule's direction,
-#: the boundary that decides a lone call, and what that lone call must still say), and a drift in
-#: any of them still moves both — which is the property the comparison exists for.
-SHARED_UNIT = "every call you can already specify"
+#: session's prefix, so the two state the same rule at different lengths — R10's `issue every call you
+#: can already specify, in one response: sending several tool calls in one round … is the normal case`
+#: against the bullet's `Every reading you can already specify goes out in one response; a call stands
+#: alone only when its arguments need an answer you have not read yet`.
+#:
+#: The marker set moved on 2026-10-21 (issue #2437), when the bullet's subject changed from a count of
+#: calls to a plan of the round, and it moved because a marker that one side dropped cannot be a phrase
+#: two carriers share. What went is `names the specific answer it waits for`: the rewritten bullet does
+#: not restate it, so that clause now lives in R10 alone, and by that alone it is not a shared phrase —
+#: a comparison holding it would report every future rewrite as a drift. What came in is the reason
+#: (`emits fresh output`), which both carriers do still state word for word. The ruled-out alternative
+#: was to keep the old marker and edit R10 in the same PR: the host scoped this change to the bullet,
+#: and a rulebook entry rewritten to fit a one-line prompt change is the drift this test exists to stop.
+SHARED_UNIT = "you can already specify"
 SHARED_CRITERION = "its arguments need an answer you have not read yet"
-SHARED_BOUNDARY = "names the specific answer it waits for"
+SHARED_REASON = "emits fresh output"
 
 #: The unit this rule may no longer be stated in. The first version of R10 taught exactly
 #: these; the host retired the merge-several-calls-into-one reading explicitly at
@@ -185,7 +192,7 @@ def test_both_carriers_state_the_same_unit_and_criterion() -> None:
         "emrg/server/prompts/system.j2": SYSTEM_PROMPT.read_text(encoding="utf-8"),
     }
     for marker, label in ((SHARED_UNIT, "unit"), (SHARED_CRITERION, "criterion"),
-                          (SHARED_BOUNDARY, "boundary")):
+                          (SHARED_REASON, "reason")):
         missing = [name for name, text in carriers.items() if marker not in text]
         assert not missing, (
             f"the {label} `{marker}` is not in {missing}: R10 and system.j2 carry one rule, so "