forked from enderofwings/NexusOS
AthenaandClaude Sonnet 5
0bbe5e200e
Gives the assistant three new ACTION tools to change its own playbooks,
runtime settings, and (source checkout only) its own source code, all
reusing the existing run_snippet/remember approval framework — but with
a hardcoded floor (self_edit.ALWAYS_ASK_TOOLS) so these three always pause
for per-call human approval regardless of the global action_tool_policy
setting. Flipping that policy for an unrelated tool must never silently
also unlock unattended self-modification.
synapse/self_edit.py is the new module doing the actual work, documented
in the same explicit "here's what is and isn't a security boundary" style
as code_run.py:
- edit_source is confined to settings.project_root via the same
realpath + Path.parents boundary check that just closed a sibling-
directory bypass in /icons/image, plus a denylist of dangerous
subtrees (.git, the venv, node_modules, build output, runtime state).
Gated on settings.source_checkout — refuses cleanly in a wheel
install, where there's no live repo to edit or commit into.
- The model sends full file content, never a diff; the server computes
the diff itself via difflib against what's actually on disk, so a
human reviews ground truth, not a description the model wrote.
- Every applied source edit best-effort commits to git as an audit
trail — independent of, not a substitute for, the approval gate.
- edit_playbook merges instead of replacing (main.py's prior
_persist_playbook did a raw replace, which was only safe because the
frontend form always sent a complete object — unsafe for a tool a
model calls with a partial argument set, so this also fixes that
latent bug). Becoming the active system prompt requires an explicit
make_active flag, never a side effect of an ordinary edit.
- edit_settings reuses the existing _SETTINGS_DEFAULTS allowlist.
The approval UI (Chatbot.jsx) previously rendered a tool call's arguments
as Object.values(args).join(", ") in a single-line badge — unusable for
reviewing a diff. It now renders a real, server-computed preview (diff
for source, before/after for playbook/settings) via a new shared
diff-view.js helper, with a loud banner when a change would become the
active system prompt or touch action_tool_policy/system_prompt. A new
nexus-edit fence (self-edit-langs.js + Markdown.jsx's EditBlock) shows
the same diff after an edit is applied, mirroring nexus-run.
Verified: 212 backend tests pass (29 new in test_self_edit.py; the 12
pre-existing C/C++/Rust toolchain failures are unrelated and unchanged),
57 frontend node:test cases pass (15 new), eslint and vite build clean,
and the full approval-preview render path was exercised against the real
built UI with a mocked SSE stream covering all four preview branches
(source diff, playbook becomes-main, settings policy-change, and a
rejected/failing preview).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
309 lines
13 KiB
Python
309 lines
13 KiB
Python
"""Self-modification: synapse/self_edit.py, plus the ACTION_TOOLS/approval-floor
|
|
wiring in tools.py and chat.py that gates it.
|
|
|
|
Path-boundary tests mirror the sibling-directory-bypass idiom already used for
|
|
/icons/image (tests/test_smoke.py) and icons/compositor.py — the same bug class,
|
|
fixed the same way, tested the same way.
|
|
"""
|
|
import asyncio
|
|
import json
|
|
import shutil
|
|
import subprocess
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
from synapse import self_edit
|
|
from synapse import tools
|
|
from synapse import playbook_manager
|
|
from synapse.nexus_config import settings
|
|
from synapse.playbooks.store import PlaybookFileStore, PlaybookItem
|
|
|
|
|
|
def _requires_git():
|
|
return pytest.mark.skipif(not shutil.which("git"), reason="git not installed")
|
|
|
|
|
|
@pytest.fixture
|
|
def fake_project(tmp_path, monkeypatch):
|
|
"""A throwaway project root with the subdirs edit_source must know about."""
|
|
root = tmp_path / "proj"
|
|
(root / "synapse").mkdir(parents=True)
|
|
(root / ".git").mkdir()
|
|
(root / "runtime").mkdir()
|
|
(root / "data").mkdir()
|
|
monkeypatch.setattr(settings, "project_root", root)
|
|
monkeypatch.setattr(settings, "source_checkout", True)
|
|
return root
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Path boundary
|
|
# ---------------------------------------------------------------------------
|
|
|
|
def test_resolve_source_path_allows_a_file_under_root(fake_project):
|
|
f = fake_project / "synapse" / "foo.py"
|
|
f.write_text("x", encoding="utf-8")
|
|
resolved = self_edit._resolve_source_path("synapse/foo.py")
|
|
assert resolved == f.resolve()
|
|
|
|
|
|
def test_resolve_source_path_denies_sibling_directory_bypass(fake_project, tmp_path):
|
|
# A sibling directory that merely shares a string prefix with the allowed
|
|
# root ("proj-evil" vs "proj") must not pass — the exact bug class fixed
|
|
# today in main.py's /icons/image.
|
|
sibling = tmp_path / "proj-evil"
|
|
sibling.mkdir()
|
|
(sibling / "x.py").write_text("evil", encoding="utf-8")
|
|
with pytest.raises(self_edit.PathError):
|
|
self_edit._resolve_source_path("../proj-evil/x.py")
|
|
|
|
|
|
@pytest.mark.parametrize("subdir", sorted(self_edit._DENYLIST_SUBDIRS))
|
|
def test_resolve_source_path_denies_each_denylisted_subdir(fake_project, subdir):
|
|
with pytest.raises(self_edit.PathError):
|
|
self_edit._resolve_source_path(f"{subdir}/whatever.txt")
|
|
|
|
|
|
def test_resolve_source_path_denies_absolute_paths(fake_project):
|
|
with pytest.raises(self_edit.PathError):
|
|
self_edit._resolve_source_path("C:\\Windows\\System32\\evil.py")
|
|
with pytest.raises(self_edit.PathError):
|
|
self_edit._resolve_source_path("/etc/passwd")
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# source_checkout gating
|
|
# ---------------------------------------------------------------------------
|
|
|
|
def test_source_checkout_gating(fake_project, monkeypatch):
|
|
monkeypatch.setattr(settings, "source_checkout", False)
|
|
with pytest.raises(RuntimeError):
|
|
self_edit.source_checkout_required()
|
|
preview = self_edit.preview_source_edit("synapse/foo.py", "x")
|
|
assert preview["ok"] is False
|
|
assert "not a source checkout" in preview["error"]
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Diff computed from ground truth
|
|
# ---------------------------------------------------------------------------
|
|
|
|
def test_preview_computes_a_real_diff_not_the_models_claim(fake_project):
|
|
f = fake_project / "synapse" / "foo.py"
|
|
f.write_text("print('old')\n", encoding="utf-8")
|
|
preview = self_edit.preview_source_edit("synapse/foo.py", "print('new')\n")
|
|
assert preview["ok"] is True
|
|
assert "-print('old')" in preview["diff"]
|
|
assert "+print('new')" in preview["diff"]
|
|
|
|
|
|
def test_preview_rejects_oversized_content(fake_project):
|
|
huge = "x" * (self_edit.MAX_FILE_CHARS + 1)
|
|
preview = self_edit.preview_source_edit("synapse/foo.py", huge)
|
|
assert preview["ok"] is False
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Apply + git commit
|
|
# ---------------------------------------------------------------------------
|
|
|
|
@_requires_git()
|
|
def test_apply_source_edit_writes_and_commits(fake_project):
|
|
subprocess.run(["git", "init"], cwd=fake_project, check=True, capture_output=True)
|
|
subprocess.run(["git", "config", "user.email", "test@test"], cwd=fake_project, check=True, capture_output=True)
|
|
subprocess.run(["git", "config", "user.name", "test"], cwd=fake_project, check=True, capture_output=True)
|
|
|
|
result = self_edit.apply_source_edit("synapse/foo.py", "print('applied')\n", "add foo")
|
|
assert result["ok"] is True
|
|
assert (fake_project / "synapse" / "foo.py").read_text(encoding="utf-8") == "print('applied')\n"
|
|
assert result["commit"] is not None
|
|
|
|
log = subprocess.run(["git", "log", "--oneline"], cwd=fake_project, check=True,
|
|
capture_output=True, text=True)
|
|
assert "self-edit: add foo" in log.stdout
|
|
|
|
|
|
def test_apply_source_edit_degrades_to_no_commit_when_git_fails(fake_project, monkeypatch):
|
|
# No `git init` here — fake_project/.git exists as a plain directory, not a
|
|
# real repo, so git commands fail. The write must still succeed.
|
|
result = self_edit.apply_source_edit("synapse/foo.py", "print('ok')\n", "x")
|
|
assert result["ok"] is True
|
|
assert result["commit"] is None
|
|
assert (fake_project / "synapse" / "foo.py").read_text(encoding="utf-8") == "print('ok')\n"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Playbook merge semantics
|
|
# ---------------------------------------------------------------------------
|
|
|
|
@pytest.fixture
|
|
def fake_playbooks(tmp_path, monkeypatch):
|
|
store = PlaybookFileStore(tmp_path / "playbooks")
|
|
monkeypatch.setattr(playbook_manager, "playbook_store", store)
|
|
monkeypatch.setattr(self_edit, "playbook_store", store)
|
|
return store
|
|
|
|
|
|
def test_edit_playbook_merge_preserves_omitted_fields(fake_playbooks):
|
|
fake_playbooks.add_playbook(PlaybookItem(
|
|
id="p1", title="Title", goal="Goal", instructions="Do X",
|
|
tags=["a"], tools=["remember"], model="m1", order=0,
|
|
))
|
|
result = playbook_manager.persist_playbook({"id": "p1", "instructions": "Do Y"}, merge=True)
|
|
assert result["instructions"] == "Do Y"
|
|
assert result["title"] == "Title"
|
|
assert result["goal"] == "Goal"
|
|
assert result["tags"] == ["a"]
|
|
assert result["tools"] == ["remember"]
|
|
assert result["model"] == "m1"
|
|
|
|
|
|
def test_edit_playbook_requires_full_fields_for_new(fake_playbooks):
|
|
with pytest.raises(ValueError):
|
|
playbook_manager.persist_playbook({"instructions": "only this"}, merge=True)
|
|
|
|
|
|
def test_edit_playbook_create_appends_at_tail_not_main(fake_playbooks):
|
|
fake_playbooks.add_playbook(PlaybookItem(id="p1", title="A", goal="g", instructions="i", order=0))
|
|
result = playbook_manager.persist_playbook(
|
|
{"title": "B", "goal": "g2", "instructions": "i2"}, merge=True,
|
|
)
|
|
assert result["order"] != 0
|
|
main = fake_playbooks.all_playbooks()[0]
|
|
assert main.id == "p1"
|
|
|
|
|
|
def test_make_main_reassigns_order_zero_without_corrupting_the_rest(fake_playbooks):
|
|
fake_playbooks.add_playbook(PlaybookItem(id="p1", title="A", goal="g", instructions="i", order=0))
|
|
fake_playbooks.add_playbook(PlaybookItem(id="p2", title="B", goal="g", instructions="i", order=1))
|
|
playbook_manager.make_main("p2")
|
|
all_pb = fake_playbooks.all_playbooks()
|
|
assert all_pb[0].id == "p2"
|
|
assert {p.id for p in all_pb} == {"p1", "p2"}
|
|
|
|
|
|
def test_preview_playbook_edit_flags_becomes_main(fake_playbooks):
|
|
fake_playbooks.add_playbook(PlaybookItem(id="p1", title="A", goal="g", instructions="i", order=0))
|
|
fake_playbooks.add_playbook(PlaybookItem(id="p2", title="B", goal="g", instructions="i", order=1))
|
|
preview = self_edit.preview_playbook_edit({"id": "p2", "make_active": True})
|
|
assert preview["ok"] is True
|
|
assert preview["becomes_main_playbook"] is True
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Settings edit
|
|
# ---------------------------------------------------------------------------
|
|
|
|
def test_edit_settings_ignores_unknown_keys():
|
|
preview = self_edit.preview_settings_edit({"changes": {"model": "llama3.1:8b", "not_a_real_key": 1}})
|
|
assert preview["ok"] is True
|
|
assert "model" in preview["applied"]
|
|
assert "not_a_real_key" in preview["ignored_unknown"]
|
|
|
|
|
|
def test_edit_settings_flags_policy_and_system_prompt_changes():
|
|
preview = self_edit.preview_settings_edit({"changes": {"action_tool_policy": "allow"}})
|
|
assert preview["policy_change"] is True
|
|
preview2 = self_edit.preview_settings_edit({"changes": {"system_prompt": "be nice"}})
|
|
assert preview2["system_prompt_change"] is True
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Approval floor + payload enrichment (chat.py wiring)
|
|
# ---------------------------------------------------------------------------
|
|
|
|
class _EditManager:
|
|
"""Returns one edit_settings tool_call, then plain content."""
|
|
def __init__(self, args):
|
|
self.n = 0
|
|
self.args = args
|
|
|
|
async def chat(self, **_):
|
|
self.n += 1
|
|
if self.n == 1:
|
|
return {"role": "assistant",
|
|
"tool_calls": [{"function": {"name": "edit_settings", "arguments": self.args}}]}
|
|
return {"role": "assistant", "content": "done"}
|
|
|
|
|
|
def _drive(policy, decision, args, monkeypatch, preview_stub=None):
|
|
from synapse import chat as chatmod
|
|
|
|
async def fake_dispatch(name, call_args):
|
|
return json.dumps({"ok": True})
|
|
monkeypatch.setattr(tools, "dispatch", fake_dispatch)
|
|
|
|
if preview_stub is not None:
|
|
monkeypatch.setattr(self_edit, "preview_for", lambda name, a: preview_stub)
|
|
|
|
async def run():
|
|
messages = [{"role": "user", "content": "change a setting"}]
|
|
schemas = tools.schemas_for(["edit_settings"])
|
|
gen = chatmod._run_tool_loop(_EditManager(args), messages, "m", schemas, None, None,
|
|
conversation_id="conv2", policy=policy)
|
|
statuses = []
|
|
approve_payload = None
|
|
async for s in gen:
|
|
statuses.append(s)
|
|
if s.startswith("__approve__"):
|
|
approve_payload = json.loads(s[len("__approve__"):])
|
|
w = chatmod.pending_approvals["conv2"]
|
|
w["decisions"] = {"edit_settings": decision}
|
|
w["event"].set()
|
|
return statuses, approve_payload
|
|
|
|
return asyncio.run(run())
|
|
|
|
|
|
def test_edit_settings_requires_approval_even_when_policy_is_allow(monkeypatch):
|
|
statuses, payload = _drive("allow", True, {"changes": {"model": "x"}}, monkeypatch)
|
|
assert any(s.startswith("__approve__") for s in statuses)
|
|
assert payload is not None
|
|
|
|
|
|
def test_approve_payload_carries_the_computed_preview(monkeypatch):
|
|
stub = {"ok": True, "applied": {"model": {"before": "a", "after": "x"}}}
|
|
statuses, payload = _drive("ask", True, {"changes": {"model": "x"}}, monkeypatch, preview_stub=stub)
|
|
assert payload["actions"][0]["preview"] == stub
|
|
|
|
|
|
def test_approve_payload_degrades_gracefully_when_preview_raises(monkeypatch):
|
|
def boom(name, args):
|
|
raise RuntimeError("preview exploded")
|
|
monkeypatch.setattr(self_edit, "preview_for", boom)
|
|
statuses, payload = _drive("ask", True, {"changes": {"model": "x"}}, monkeypatch)
|
|
assert payload is not None
|
|
assert payload["actions"][0]["preview"]["ok"] is False
|
|
|
|
|
|
def test_action_tools_include_self_edit_and_are_gated_by_consent():
|
|
allow = ["edit_playbook", "edit_settings", "edit_source"]
|
|
on = [s["function"]["name"] for s in tools.schemas_for(allow, allow_actions=True)]
|
|
off = [s["function"]["name"] for s in tools.schemas_for(allow, allow_actions=False)]
|
|
assert set(on) == set(allow)
|
|
assert off == []
|
|
for name in allow:
|
|
assert tools.is_action(name)
|
|
assert name in self_edit.ALWAYS_ASK_TOOLS
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# End-to-end: dispatch("edit_source", ...) through the real preview/apply path
|
|
# ---------------------------------------------------------------------------
|
|
|
|
@_requires_git()
|
|
def test_dispatch_edit_source_end_to_end(fake_project):
|
|
subprocess.run(["git", "init"], cwd=fake_project, check=True, capture_output=True)
|
|
subprocess.run(["git", "config", "user.email", "test@test"], cwd=fake_project, check=True, capture_output=True)
|
|
subprocess.run(["git", "config", "user.name", "test"], cwd=fake_project, check=True, capture_output=True)
|
|
|
|
out = asyncio.run(tools.dispatch("edit_source", {
|
|
"path": "synapse/foo.py", "new_content": "print('e2e')\n", "summary": "e2e test",
|
|
}))
|
|
payload = json.loads(out)
|
|
assert payload["ok"] is True
|
|
assert payload["commit"] is not None
|
|
assert "nexus-edit" in payload["fence"]
|
|
assert (fake_project / "synapse" / "foo.py").read_text(encoding="utf-8") == "print('e2e')\n"
|
GitNexus · part of the Nexus stack