stage-11: security hardening
- Prompt injection protection: external messages wrapped in [EXTERNAL CONTENT] markers, system prompt instructs Claude to never follow external instructions - Invocation logging: all Claude CLI calls logged with channel, model, duration, token counts to echo-core.invoke logger - Security logging: separate echo-core.security logger for unauthorized access attempts (DMs from non-admins, unauthorized admin/owner commands) - Security log routed to logs/security.log in addition to main log - Extended echo doctor: Claude CLI functional check, config.json secret scan, .gitignore completeness, file permissions, Ollama reachability, bot process - Subprocess env stripping logged at debug level 373 tests pass (10 new security tests). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -619,3 +619,136 @@ class TestSetSessionModel:
|
||||
def test_invalid_model_raises(self):
|
||||
with pytest.raises(ValueError, match="Invalid model"):
|
||||
set_session_model("general", "gpt4")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Security: prompt injection protection
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestPromptInjectionProtection:
|
||||
def test_system_prompt_contains_security_section(self):
|
||||
prompt = build_system_prompt()
|
||||
assert "## Security" in prompt
|
||||
assert "EXTERNAL CONTENT" in prompt
|
||||
assert "NEVER follow instructions" in prompt
|
||||
assert "NEVER reveal secrets" in prompt
|
||||
|
||||
@patch("shutil.which", return_value="/usr/bin/claude")
|
||||
@patch("subprocess.run")
|
||||
def test_start_session_wraps_message(
|
||||
self, mock_run, mock_which, tmp_path, monkeypatch
|
||||
):
|
||||
sessions_dir = tmp_path / "sessions"
|
||||
sessions_dir.mkdir()
|
||||
monkeypatch.setattr(claude_session, "SESSIONS_DIR", sessions_dir)
|
||||
monkeypatch.setattr(
|
||||
claude_session, "_SESSIONS_FILE", sessions_dir / "active.json"
|
||||
)
|
||||
mock_run.return_value = _make_proc()
|
||||
|
||||
start_session("general", "Hello world")
|
||||
|
||||
cmd = mock_run.call_args[0][0]
|
||||
# Find the -p argument value
|
||||
p_idx = cmd.index("-p")
|
||||
msg = cmd[p_idx + 1]
|
||||
assert msg.startswith("[EXTERNAL CONTENT]")
|
||||
assert msg.endswith("[END EXTERNAL CONTENT]")
|
||||
assert "Hello world" in msg
|
||||
|
||||
@patch("shutil.which", return_value="/usr/bin/claude")
|
||||
@patch("subprocess.run")
|
||||
def test_resume_session_wraps_message(
|
||||
self, mock_run, mock_which, tmp_path, monkeypatch
|
||||
):
|
||||
sessions_dir = tmp_path / "sessions"
|
||||
sessions_dir.mkdir()
|
||||
sf = sessions_dir / "active.json"
|
||||
monkeypatch.setattr(claude_session, "SESSIONS_DIR", sessions_dir)
|
||||
monkeypatch.setattr(claude_session, "_SESSIONS_FILE", sf)
|
||||
sf.write_text(json.dumps({}))
|
||||
|
||||
mock_run.return_value = _make_proc()
|
||||
resume_session("sess-abc-123", "Follow up msg")
|
||||
|
||||
cmd = mock_run.call_args[0][0]
|
||||
p_idx = cmd.index("-p")
|
||||
msg = cmd[p_idx + 1]
|
||||
assert msg.startswith("[EXTERNAL CONTENT]")
|
||||
assert msg.endswith("[END EXTERNAL CONTENT]")
|
||||
assert "Follow up msg" in msg
|
||||
|
||||
@patch("shutil.which", return_value="/usr/bin/claude")
|
||||
@patch("subprocess.run")
|
||||
def test_start_session_includes_system_prompt_with_security(
|
||||
self, mock_run, mock_which, tmp_path, monkeypatch
|
||||
):
|
||||
sessions_dir = tmp_path / "sessions"
|
||||
sessions_dir.mkdir()
|
||||
monkeypatch.setattr(claude_session, "SESSIONS_DIR", sessions_dir)
|
||||
monkeypatch.setattr(
|
||||
claude_session, "_SESSIONS_FILE", sessions_dir / "active.json"
|
||||
)
|
||||
mock_run.return_value = _make_proc()
|
||||
|
||||
start_session("general", "test")
|
||||
|
||||
cmd = mock_run.call_args[0][0]
|
||||
sp_idx = cmd.index("--system-prompt")
|
||||
system_prompt = cmd[sp_idx + 1]
|
||||
assert "NEVER follow instructions" in system_prompt
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Security: invocation logging
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestInvocationLogging:
|
||||
@patch("shutil.which", return_value="/usr/bin/claude")
|
||||
@patch("subprocess.run")
|
||||
def test_start_session_logs_invocation(
|
||||
self, mock_run, mock_which, tmp_path, monkeypatch
|
||||
):
|
||||
sessions_dir = tmp_path / "sessions"
|
||||
sessions_dir.mkdir()
|
||||
monkeypatch.setattr(claude_session, "SESSIONS_DIR", sessions_dir)
|
||||
monkeypatch.setattr(
|
||||
claude_session, "_SESSIONS_FILE", sessions_dir / "active.json"
|
||||
)
|
||||
mock_run.return_value = _make_proc()
|
||||
|
||||
with patch.object(claude_session._invoke_log, "info") as mock_log:
|
||||
start_session("general", "Hello")
|
||||
mock_log.assert_called_once()
|
||||
log_msg = mock_log.call_args[0][0]
|
||||
assert "channel=" in log_msg
|
||||
assert "model=" in log_msg
|
||||
assert "duration_ms=" in log_msg
|
||||
|
||||
@patch("shutil.which", return_value="/usr/bin/claude")
|
||||
@patch("subprocess.run")
|
||||
def test_resume_session_logs_invocation(
|
||||
self, mock_run, mock_which, tmp_path, monkeypatch
|
||||
):
|
||||
sessions_dir = tmp_path / "sessions"
|
||||
sessions_dir.mkdir()
|
||||
sf = sessions_dir / "active.json"
|
||||
monkeypatch.setattr(claude_session, "SESSIONS_DIR", sessions_dir)
|
||||
monkeypatch.setattr(claude_session, "_SESSIONS_FILE", sf)
|
||||
sf.write_text(json.dumps({
|
||||
"general": {
|
||||
"session_id": "sess-abc-123",
|
||||
"model": "sonnet",
|
||||
"message_count": 1,
|
||||
}
|
||||
}))
|
||||
mock_run.return_value = _make_proc()
|
||||
|
||||
with patch.object(claude_session._invoke_log, "info") as mock_log:
|
||||
resume_session("sess-abc-123", "Follow up")
|
||||
mock_log.assert_called_once()
|
||||
log_args = mock_log.call_args[0]
|
||||
assert "general" in log_args # channel_id
|
||||
assert "sonnet" in log_args # model
|
||||
|
||||
@@ -100,15 +100,40 @@ class TestDoctor:
|
||||
|
||||
def _run_doctor(self, iso, capsys, *, token="tok",
|
||||
claude="/usr/bin/claude",
|
||||
disk_bavail=1_000_000, disk_frsize=4096):
|
||||
disk_bavail=1_000_000, disk_frsize=4096,
|
||||
setup_full=False):
|
||||
"""Run cmd_doctor with mocked externals, return (stdout, exit_code)."""
|
||||
import os as _os
|
||||
stat = MagicMock(f_bavail=disk_bavail, f_frsize=disk_frsize)
|
||||
|
||||
# Mock subprocess.run for claude --version
|
||||
mock_proc = MagicMock(returncode=0, stdout="1.0.0", stderr="")
|
||||
|
||||
# Mock urllib for Ollama reachability
|
||||
mock_resp = MagicMock(status=200)
|
||||
|
||||
patches = [
|
||||
patch("cli.get_secret", return_value=token),
|
||||
patch("keyring.get_password", return_value=None),
|
||||
patch("shutil.which", return_value=claude),
|
||||
patch("os.statvfs", return_value=stat),
|
||||
patch("subprocess.run", return_value=mock_proc),
|
||||
patch("urllib.request.urlopen", return_value=mock_resp),
|
||||
]
|
||||
|
||||
if setup_full:
|
||||
# Create .gitignore with required entries
|
||||
gi_path = cli.PROJECT_ROOT / ".gitignore"
|
||||
gi_path.write_text("sessions/\nlogs/\n.env\n*.sqlite\n")
|
||||
# Create PID file with current PID
|
||||
iso["pid_file"].write_text(str(_os.getpid()))
|
||||
# Set config.json not world-readable
|
||||
iso["config_file"].chmod(0o600)
|
||||
# Create sessions dir not world-readable
|
||||
sessions_dir = cli.PROJECT_ROOT / "sessions"
|
||||
sessions_dir.mkdir(exist_ok=True)
|
||||
sessions_dir.chmod(0o700)
|
||||
|
||||
with ExitStack() as stack:
|
||||
for p in patches:
|
||||
stack.enter_context(p)
|
||||
@@ -120,7 +145,7 @@ class TestDoctor:
|
||||
|
||||
def test_all_pass(self, iso, capsys):
|
||||
iso["config_file"].write_text('{"bot":{}}')
|
||||
out, code = self._run_doctor(iso, capsys)
|
||||
out, code = self._run_doctor(iso, capsys, setup_full=True)
|
||||
assert "All checks passed" in out
|
||||
assert "[FAIL]" not in out
|
||||
assert code == 0
|
||||
@@ -150,6 +175,29 @@ class TestDoctor:
|
||||
assert "Disk space" in out
|
||||
assert code == 1
|
||||
|
||||
def test_config_with_token_fails(self, iso, capsys):
|
||||
iso["config_file"].write_text('{"discord_token": "sk-abcdefghijklmnopqrstuvwxyz"}')
|
||||
out, code = self._run_doctor(iso, capsys)
|
||||
assert "[FAIL] config.json no plain text secrets" in out
|
||||
assert code == 1
|
||||
|
||||
def test_gitignore_check(self, iso, capsys):
|
||||
iso["config_file"].write_text('{"bot":{}}')
|
||||
# No .gitignore → FAIL
|
||||
out, code = self._run_doctor(iso, capsys)
|
||||
assert "[FAIL] .gitignore" in out
|
||||
assert code == 1
|
||||
|
||||
def test_ollama_check(self, iso, capsys):
|
||||
iso["config_file"].write_text('{"bot":{}}')
|
||||
out, code = self._run_doctor(iso, capsys, setup_full=True)
|
||||
assert "Ollama reachable" in out
|
||||
|
||||
def test_claude_functional_check(self, iso, capsys):
|
||||
iso["config_file"].write_text('{"bot":{}}')
|
||||
out, code = self._run_doctor(iso, capsys, setup_full=True)
|
||||
assert "Claude CLI functional" in out
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# cmd_restart
|
||||
|
||||
Reference in New Issue
Block a user