From 98a967625f81f913bd8d2f8f5af42993ac319097 Mon Sep 17 00:00:00 2001 From: pufit Date: Sun, 9 Aug 2026 10:24:04 -0400 Subject: [PATCH] Never put the system prompt on the command line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The assembled system prompt is the instance's private context — identity and memory files plus TOOLS.md, which indexes where the host's credentials live. It was passed to the CLI as `--system-prompt ` whenever it fit under a 100 KB threshold, which put the whole bundle in argv: readable by any local process, for every concurrent session, with one unprivileged `ps` and no trace. Over the threshold it spilled to a file instead — but at 0644 in a 0755 dir, so that branch only moved the exposure to disk. Which side a session landed on was incidental drift. Now it always travels by file, at every size, written 0600 in a 0700 directory via the atomic writer (mode set before the content lands). The sweep that GCs stale spills also clamps the ones earlier versions left at 0644, so existing installs are remediated on next session start, and it lstats rather than stats so a planted symlink can't redirect it. Prompt-cache behavior is keyed on prompt content, not transport: over the last 7 days on a live instance the file path showed a 96.1% cache-read share against 91.9% for inline, so there is nothing to trade off and no reason to keep a knob whose only other setting is "leak". Side effects: the E2BIG failure the threshold was originally guarding against can no longer happen, and workspace text is no longer a `pkill -f` pattern matching every session on the box. Measured: 101,683 bytes of argv -> 428. --- nerve/agent/backends/base.py | 14 +- nerve/agent/backends/claude.py | 146 +++++++++----- tests/test_engine.py | 10 +- tests/test_system_prompt_transport.py | 280 ++++++++++++++++++++++++++ 4 files changed, 398 insertions(+), 52 deletions(-) create mode 100644 tests/test_system_prompt_transport.py diff --git a/nerve/agent/backends/base.py b/nerve/agent/backends/base.py index bad960a0..5d0d2112 100644 --- a/nerve/agent/backends/base.py +++ b/nerve/agent/backends/base.py @@ -53,7 +53,19 @@ class TurnInput: @dataclass class SessionSpec: - """Everything a backend needs to build a client for one session.""" + """Everything a backend needs to build a client for one session. + + **``system_prompt`` never goes on a command line.** It carries the + instance's private context — the operator's identity and memory files + plus TOOLS.md, which indexes where the host's credentials live — and + argv is world-readable: one unprivileged ``ps`` reads it out of every + running session at once. Backends must hand it to their runtime out + of band. Claude writes it to a 0600 file and passes the path + (``--system-prompt-file``); Codex sends it as ``developerInstructions`` + inside the app-server JSON payload. A new backend that puts it in an + argument is a data leak, not a style choice — + ``tests/test_system_prompt_transport.py`` guards the invariant. + """ session_id: str source: str diff --git a/nerve/agent/backends/claude.py b/nerve/agent/backends/claude.py index e8578dc4..cee8eebf 100644 --- a/nerve/agent/backends/claude.py +++ b/nerve/agent/backends/claude.py @@ -25,6 +25,9 @@ import logging import os import re +import stat +import time +from pathlib import Path from typing import Any, AsyncIterator from claude_agent_sdk import ( @@ -63,6 +66,7 @@ InteractiveToolHandler, _read_file_safe, ) +from nerve.utils.fs import atomic_write_text logger = logging.getLogger(__name__) @@ -71,21 +75,36 @@ except ImportError: # pragma: no cover - depends on SDK version ThinkingBlock = None -# Linux execve() limits a single argv element to MAX_ARG_STRLEN = PAGE_SIZE * 32 -# = 131,072 bytes on common configurations. The Claude Agent SDK passes the -# system prompt inline as `--system-prompt `, which makes the string a -# single argv element. When SOUL.md + TASK.md + AGENTS.md + TOOLS.md + -# MEMORY.md + recalled memU summaries cross that boundary, execve() returns -# E2BIG ("Argument list too long") and Claude Code fails to start. +# The system prompt is confidential, so it never travels in argv. # -# We sidestep the limit by writing the prompt to a file and passing -# `SystemPromptFile = {"type": "file", "path": ...}` (which the SDK converts -# to `--system-prompt-file ` — the path string is short). +# What Nerve assembles into it is the instance's whole private context: +# SOUL.md / IDENTITY.md / USER.md (the operator's name, addresses, health +# notes), AGENTS.md and TOOLS.md (which is an index of where every +# credential on the host lives), MEMORY.md, and the recalled-memory block. +# The SDK's default transport is `--system-prompt `, and argv is +# world-readable on every OS Nerve runs on: any process under any uid can +# `ps -ww` the lot out of a running session, for every session at once, +# unprivileged and untraced. Every MCP server Nerve spawns and every +# command an agent shells out to is such a process. # -# Threshold below which we keep passing inline (preserves prompt-cache hit -# behavior for small, stable prompts). Set conservatively well under the -# kernel limit to leave room for env/argv overhead. -SYSTEM_PROMPT_INLINE_MAX = 100_000 # bytes +# So `_build_options` always passes `SystemPromptFile = {"type": "file", +# "path": ...}` — the SDK turns that into `--system-prompt-file `, +# and only the short path reaches the process table. The file itself is +# written 0600 in a 0700 directory (see ``_write_system_prompt_file``). +# There is no size threshold and no inline branch: a small prompt is not +# less secret than a large one, and prompt-cache behavior is keyed on the +# prompt's *content*, not on how it reached the CLI. +# +# Two consequences worth naming, both of which used to be the whole +# rationale for this path: +# +# * execve() caps a single argv element at MAX_ARG_STRLEN (PAGE_SIZE * 32 +# = 131,072 bytes on common Linux configs). A full bundle crosses that +# and the CLI fails to start with E2BIG. Off argv, it cannot happen. +# * Workspace-file text in argv makes every string in SOUL/TOOLS/MEMORY a +# `pkill -f` pattern that matches *every* concurrent session on the +# box. Off argv, an ordinary cleanup command stops being a fleet-wide +# kill switch. # CLI env vars that remap model *aliases* (the short names accepted by the # Agent/Workflow tools' model option, skill frontmatter, `--model opus`, @@ -441,20 +460,14 @@ def _build_options(self, spec: SessionSpec) -> ClaudeAgentOptions: config = self.config session_id = spec.session_id - # Pass the system prompt as a file when it's large enough to risk - # hitting Linux's MAX_ARG_STRLEN argv-element limit (see the - # SYSTEM_PROMPT_INLINE_MAX comment above). - system_prompt: str | dict[str, Any] - if len(spec.system_prompt) > SYSTEM_PROMPT_INLINE_MAX: - sp_path = self._write_system_prompt_file(session_id, spec.system_prompt) - system_prompt = {"type": "file", "path": sp_path} - logger.info( - "Session %s: system prompt %d bytes (> %d), passing via file %s", - session_id[:8], len(spec.system_prompt), - SYSTEM_PROMPT_INLINE_MAX, sp_path, - ) - else: - system_prompt = spec.system_prompt + # Always via file, never inline — argv is world-readable and this + # prompt is the instance's private context (see the module comment). + sp_path = self._write_system_prompt_file(session_id, spec.system_prompt) + system_prompt: dict[str, Any] = {"type": "file", "path": sp_path} + logger.debug( + "Session %s: system prompt %d bytes passed via file %s", + session_id[:8], len(spec.system_prompt), sp_path, + ) # Local Ollama models are reached through the proxy and speak the # OpenAI-translated API — Anthropic-only knobs (extended thinking, @@ -565,39 +578,76 @@ def _cli_stderr(line: str) -> None: plugins=self._deps.claude_plugins(), ) - def _system_prompt_dir(self) -> "os.PathLike[str]": - """Directory where oversized system prompts are spilled to disk.""" - from pathlib import Path + def _system_prompt_dir(self) -> Path: + """Owner-only directory holding the per-session system prompts. + + ``mkdir(exist_ok=True)`` does not touch the mode of a directory + that already exists, and the pre-hardening code created this one + at the umask default (0755 on most hosts) — so the chmod is + unconditional, not just part of the create. That is the upgrade + path for every install that ran the old code; without it the dir + stays world-traversable forever. + """ d = Path(self.config.workspace) / ".nerve" / "cache" / "system_prompts" - d.mkdir(parents=True, exist_ok=True) + d.mkdir(parents=True, exist_ok=True, mode=0o700) + try: + os.chmod(d, 0o700) + except OSError as e: # read-only FS, or someone else owns it + logger.warning("Could not restrict %s to 0700: %s", d, e) return d def _write_system_prompt_file(self, session_id: str, content: str) -> str: - """Write the system prompt to disk and return its absolute path. + """Write the system prompt to disk 0600 and return its path. Deterministic filename so a session that reconnects (resume) gets - the same prompt without re-writing. Lazy GC of stale files (>7d). + the same prompt without re-writing. The write is atomic and the + mode is in place *before* the content is, so the bundle is never + briefly readable at the process umask — same discipline the Codex + backend uses for the files it generates. """ - import time - from pathlib import Path + dir_path = self._system_prompt_dir() + self._sweep_system_prompt_dir(dir_path) - dir_path = Path(self._system_prompt_dir()) + safe_id = re.sub(r"[^A-Za-z0-9_.-]", "_", session_id)[:120] + path = dir_path / f"{safe_id}.md" + atomic_write_text(path, content, mode=0o600) + return str(path) + @staticmethod + def _sweep_system_prompt_dir(dir_path: Path) -> None: + """Drop spills older than 7d; clamp whatever is left to 0600. + + Two jobs, one pass over the directory. The GC is what keeps a + long-lived workspace from accumulating a prompt file per session + forever. The clamp is the upgrade path for files the + pre-hardening code already wrote at 0644: they hold the full + identity bundle and would otherwise sit there world-readable + until they aged out. + + Best-effort throughout — a file that cannot be removed or + chmod'd (foreign owner, read-only mount) must not stop a session + from starting. + + ``lstat``, not ``stat``: a symlink here is skipped rather than + followed, so neither the unlink nor the chmod can be aimed at a + file outside this directory by planting one. + """ cutoff = time.time() - 7 * 24 * 3600 try: - for old in dir_path.iterdir(): - try: - if old.is_file() and old.stat().st_mtime < cutoff: - old.unlink() - except OSError: - pass + entries = list(dir_path.iterdir()) except OSError: - pass - - safe_id = re.sub(r"[^A-Za-z0-9_.-]", "_", session_id)[:120] - path = dir_path / f"{safe_id}.md" - path.write_text(content, encoding="utf-8") - return str(path) + return + for old in entries: + try: + st = old.lstat() + if not stat.S_ISREG(st.st_mode): + continue + if st.st_mtime < cutoff: + old.unlink() + elif st.st_mode & 0o077: + os.chmod(old, 0o600) + except OSError: + pass def _build_env(self, cache_ttl: str = "5m") -> dict[str, str]: """Build environment variables for the SDK subprocess.""" diff --git a/tests/test_engine.py b/tests/test_engine.py index af135399..8d0e49e4 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -4,6 +4,7 @@ import asyncio import json import os +from pathlib import Path from types import SimpleNamespace from unittest.mock import AsyncMock, patch @@ -135,9 +136,12 @@ def test_claude_system_prompt_excludes_codex_runbook_policy(tmp_path): patch.object(backend, "_build_hooks", return_value={}): options = backend._build_options(spec) - assert options.system_prompt == marker - assert "Nerve runbooks" not in str(options.system_prompt) - assert "Codex-native skills" not in str(options.system_prompt) + # The prompt travels by file (never argv), so the exactness check is on + # what landed on disk. + rendered = Path(options.system_prompt["path"]).read_text(encoding="utf-8") + assert rendered == marker + assert "Nerve runbooks" not in rendered + assert "Codex-native skills" not in rendered # --------------------------------------------------------------------------- diff --git a/tests/test_system_prompt_transport.py b/tests/test_system_prompt_transport.py new file mode 100644 index 00000000..6ccc0775 --- /dev/null +++ b/tests/test_system_prompt_transport.py @@ -0,0 +1,280 @@ +"""The system prompt must never reach a command line, at any size. + +Nerve assembles the operator's identity and memory files plus TOOLS.md — +an index of where the host's credentials live — into every session's +system prompt. argv is world-readable: a single unprivileged ``ps -ww`` +reads it out of every running session at once. So the prompt travels out +of band, and the file it travels in is a secret's file: 0600, in a 0700 +directory, written atomically so the mode is in place before the bytes +are. + +These tests pin the transport itself (through the SDK's own argv builder, +not a shape assertion on our options object), the permissions, and the +upgrade path for installs that already spilled prompts at 0644. +""" + +import os +import time +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import patch + +import pytest + +from nerve.agent.backends.base import SessionSpec +from nerve.agent.backends.claude import ClaudeBackend +from nerve.config import NerveConfig + +# A string that appears nowhere else, standing in for the real bundle. +MARKER = "SENTINEL-a-credential-index-and-a-home-address" + + +def _backend(tmp_path: Path) -> ClaudeBackend: + cfg = NerveConfig.from_dict({"workspace": str(tmp_path)}) + return ClaudeBackend(SimpleNamespace( + config=lambda: cfg, + claude_plugins=lambda: [], + )) + + +def _spec(tmp_path: Path, prompt: str, session_id: str = "sess-1234") -> SessionSpec: + return SessionSpec( + session_id=session_id, + source="web", + model="claude-opus-5", + effort="high", + system_prompt=prompt, + cwd=str(tmp_path), + ) + + +def _build(backend: ClaudeBackend, spec: SessionSpec): + with patch.object(backend, "_build_mcp_servers", return_value={}), \ + patch.object(backend, "_build_hooks", return_value={}): + return backend._build_options(spec) + + +def _prompt_dir(tmp_path: Path) -> Path: + return tmp_path / ".nerve" / "cache" / "system_prompts" + + +# --------------------------------------------------------------------- # +# Transport # +# --------------------------------------------------------------------- # + + +@pytest.mark.parametrize("size", [0, 1, 500, 100_000, 150_000]) +def test_prompt_never_inline_at_any_size(tmp_path, size): + """No threshold, no inline branch — every size takes the file path. + + The old code inlined anything under 100 KB, which put the whole + bundle in argv for most sessions and made which-leak-you-get a + function of incidental prompt drift. + """ + prompt = MARKER + "x" * size + options = _build(_backend(tmp_path), _spec(tmp_path, prompt)) + + assert isinstance(options.system_prompt, dict) + assert options.system_prompt["type"] == "file" + assert Path(options.system_prompt["path"]).read_text(encoding="utf-8") == prompt + + +def test_sdk_argv_carries_the_path_not_the_prompt(tmp_path): + """End-to-end through the SDK's own argv builder. + + Asserting the shape of our options dict only proves we asked for the + file transport. This proves the SDK honors it — that the bytes the + kernel sees are a path, and that nothing else in the options assembly + smuggles the prompt onto the command line by another route. + """ + from claude_agent_sdk._internal.transport.subprocess_cli import ( + SubprocessCLITransport, + ) + + prompt = MARKER + "\n" + "line of private context\n" * 5000 + backend = _backend(tmp_path) + options = _build(backend, _spec(tmp_path, prompt)) + options.cli_path = "/nonexistent/claude" # skip the CLI discovery walk + + argv = SubprocessCLITransport(prompt="hi", options=options)._build_command() + + assert "--system-prompt-file" in argv + assert "--system-prompt" not in argv + # The prompt — or any fragment of it — must appear in no argument. + joined = "\x00".join(argv) + assert MARKER not in joined + assert "line of private context" not in joined + # And the whole command line stays small: a path, not a bundle. + assert len(joined) < 4096 < len(prompt) + + +def test_codex_backend_keeps_the_prompt_out_of_argv(tmp_path): + """The other backend satisfies the same invariant, differently. + + Codex sends the prompt as ``developerInstructions`` inside the + app-server JSON payload. Pinned here so the two backends cannot + silently drift apart on whether the prompt is a secret. + """ + from nerve.agent.backends.codex.backend import CodexBackend + + cfg = NerveConfig.from_dict({"workspace": str(tmp_path)}) + backend = CodexBackend(SimpleNamespace(config=lambda: cfg)) + params = backend.thread_params(_spec(tmp_path, MARKER)) + + assert MARKER in params["developerInstructions"] + # Everything else in the payload is a scalar knob; none of it is argv, + # and none of it repeats the prompt. + assert not any( + MARKER in str(v) for k, v in params.items() if k != "developerInstructions" + ) + + +# --------------------------------------------------------------------- # +# Permissions # +# --------------------------------------------------------------------- # + + +def test_spill_file_and_dir_are_owner_only(tmp_path): + backend = _backend(tmp_path) + options = _build(backend, _spec(tmp_path, MARKER)) + + path = Path(options.system_prompt["path"]) + assert path.stat().st_mode & 0o777 == 0o600 + assert path.parent.stat().st_mode & 0o777 == 0o700 + + +def test_mode_survives_a_permissive_umask(tmp_path): + """0600 by construction, not by inheriting a strict umask. + + ``open(path, "w")`` under ``umask 000`` yields 0666. If the mode came + from the umask this test would fail on exactly the hosts where it + matters most. + """ + backend = _backend(tmp_path) + old = os.umask(0o000) + try: + options = _build(backend, _spec(tmp_path, MARKER)) + finally: + os.umask(old) + + assert Path(options.system_prompt["path"]).stat().st_mode & 0o777 == 0o600 + + +def test_write_leaves_no_readable_debris(tmp_path): + """The atomic write must not leave its temp file behind. + + A stray temp file is the same leak under a different name. + """ + backend = _backend(tmp_path) + _build(backend, _spec(tmp_path, MARKER)) + _build(backend, _spec(tmp_path, MARKER + " turn two")) + + files = list(_prompt_dir(tmp_path).iterdir()) + assert len(files) == 1 + assert files[0].suffix == ".md" + assert all(f.stat().st_mode & 0o077 == 0 for f in files) + + +# --------------------------------------------------------------------- # +# Upgrade path — what earlier versions left on disk # +# --------------------------------------------------------------------- # + + +def test_existing_world_readable_dir_is_clamped(tmp_path): + """``mkdir(exist_ok=True)`` does not fix the mode of an existing dir. + + Every install that ran the pre-hardening code has this directory at + 0755. Re-creating it is a no-op; the chmod has to be unconditional. + """ + d = _prompt_dir(tmp_path) + d.mkdir(parents=True) + os.chmod(d, 0o755) + + _build(_backend(tmp_path), _spec(tmp_path, MARKER)) + + assert d.stat().st_mode & 0o777 == 0o700 + + +def test_existing_0644_spills_are_clamped(tmp_path): + """Old spills hold the full bundle and must not wait for GC to age out.""" + d = _prompt_dir(tmp_path) + d.mkdir(parents=True) + stale = d / "old-session.md" + stale.write_text("previously world-readable bundle", encoding="utf-8") + os.chmod(stale, 0o644) + + _build(_backend(tmp_path), _spec(tmp_path, MARKER)) + + assert stale.stat().st_mode & 0o777 == 0o600 + # Untouched otherwise — this is a permission fix, not a purge. + assert stale.read_text(encoding="utf-8") == "previously world-readable bundle" + + +def test_unclampable_file_does_not_block_a_session(tmp_path): + """Best-effort: a file we cannot chmod must not stop the CLI starting.""" + d = _prompt_dir(tmp_path) + d.mkdir(parents=True) + (d / "foreign.md").write_text("not ours", encoding="utf-8") + + with patch("nerve.agent.backends.claude.os.chmod", side_effect=OSError("EPERM")): + options = _build(_backend(tmp_path), _spec(tmp_path, MARKER)) + + assert Path(options.system_prompt["path"]).exists() + + +# --------------------------------------------------------------------- # +# Filename discipline and GC (unchanged behavior, still load-bearing) # +# --------------------------------------------------------------------- # + + +def test_filename_is_deterministic_across_reconnects(tmp_path): + """Resume re-uses the same path, and the content is refreshed.""" + backend = _backend(tmp_path) + first = _build(backend, _spec(tmp_path, MARKER, "sess-resume")) + second = _build(backend, _spec(tmp_path, MARKER + " v2", "sess-resume")) + + assert first.system_prompt["path"] == second.system_prompt["path"] + assert Path(second.system_prompt["path"]).read_text(encoding="utf-8").endswith("v2") + + +def test_session_id_cannot_escape_the_directory(tmp_path): + """A path-shaped session id is sanitized, not honored.""" + backend = _backend(tmp_path) + options = _build(backend, _spec(tmp_path, MARKER, "../../../../tmp/evil")) + + path = Path(options.system_prompt["path"]) + assert path.parent == _prompt_dir(tmp_path) + assert not Path("/tmp/evil.md").exists() + + +def test_sweep_does_not_follow_symlinks(tmp_path): + """A symlink in the dir must not aim the chmod (or the unlink) elsewhere.""" + d = _prompt_dir(tmp_path) + d.mkdir(parents=True) + outside = tmp_path / "someone-elses-file" + outside.write_text("not a system prompt", encoding="utf-8") + os.chmod(outside, 0o644) + link = d / "link.md" + link.symlink_to(outside) + os.utime(link, (time.time() - 8 * 24 * 3600,) * 2, follow_symlinks=False) + + _build(_backend(tmp_path), _spec(tmp_path, MARKER)) + + assert outside.exists() + assert outside.stat().st_mode & 0o777 == 0o644 + assert link.is_symlink() + + +def test_gc_prunes_stale_spills_and_keeps_fresh_ones(tmp_path): + d = _prompt_dir(tmp_path) + d.mkdir(parents=True) + old, fresh = d / "old.md", d / "fresh.md" + for f in (old, fresh): + f.write_text("x", encoding="utf-8") + eight_days = time.time() - 8 * 24 * 3600 + os.utime(old, (eight_days, eight_days)) + + _build(_backend(tmp_path), _spec(tmp_path, MARKER)) + + assert not old.exists() + assert fresh.exists()