diff --git a/docs/sleep/README.md b/docs/sleep/README.md index 3ed9d456..0ff879f3 100644 --- a/docs/sleep/README.md +++ b/docs/sleep/README.md @@ -201,6 +201,16 @@ An existing file that cannot be decoded, parsed, or read stops the command with a configuration error and exit code 2; it does not discard your settings and run with default budgets. Explicit CLI flags still override loaded values. +#### Claude replay isolation + +The `claude` replay backend requires `ANTHROPIC_API_KEY` so it can use +Claude Code's `--bare` mode and avoid user-level instructions and hooks +contaminating evaluation scores. Subscription/OAuth login cannot currently be +combined with this isolation mode; the backend stops with a clear error instead +of reporting an uncontrolled score. Use another backend when an API key is not +available. SkillOpt does not inspect the user's Claude configuration to infer +whether an API key helper is configured. + ### Harvest filters (environment) Harvest keeps what you typed and drops what a machine wrote. Four environment diff --git a/skillopt_sleep/backend.py b/skillopt_sleep/backend.py index 7134ae46..194b8fe4 100644 --- a/skillopt_sleep/backend.py +++ b/skillopt_sleep/backend.py @@ -836,6 +836,27 @@ def _detect_cli_error(self, stdout: str, stderr: str) -> None: self.last_call_error = combined[:500] return + @staticmethod + def _require_isolated_auth() -> None: + """Refuse OAuth-only CLI calls that can inherit user instructions. + + Claude Code's subscription login cannot be combined with ``--bare``. + Without that flag, a clean working directory and disabled slash + commands still do not isolate user-level instructions and hooks. Do + not inspect the user's Claude configuration to guess whether an + ``apiKeyHelper`` is present; require an explicit API key for this + backend until the CLI offers a verifiably isolated OAuth mode. + """ + if not os.environ.get("ANTHROPIC_API_KEY", "").strip(): + from skillopt_sleep.staging import StagingError + + raise StagingError( + "Claude replay refused: ANTHROPIC_API_KEY is required for isolated " + "measurement. Subscription/OAuth authentication cannot use --bare, " + "so user-level instructions and hooks may contaminate scores. " + "Configure an API key or choose another backend." + ) + def _call(self, prompt: str, *, max_tokens: int = 1024) -> str: # Run ISOLATED so the ambient Claude Code environment does not leak into # the optimizer/target call. Critically, the user's GLOBAL skills @@ -850,9 +871,9 @@ def _call(self, prompt: str, *, max_tokens: int = 1024) -> str: # --exclude-dynamic-... drop per-machine cwd/env/memory/git sections # cwd= no project CLAUDE.md import tempfile + self._require_isolated_auth() cmd = [self.claude_path, "-p", "--output-format", "text"] - if os.environ.get("ANTHROPIC_API_KEY"): - cmd.append("--bare") + cmd.append("--bare") cmd += [ "--disable-slash-commands", "--disallowedTools", "*", @@ -923,8 +944,8 @@ def attempt_with_tools(self, task, skill, memory, tools): "Return ONLY the final answer text." ) cmd = [self.claude_path, "-p", "--output-format", "text"] - if os.environ.get("ANTHROPIC_API_KEY"): - cmd.append("--bare") + self._require_isolated_auth() + cmd.append("--bare") cmd += [ "--disable-slash-commands", "--allowedTools", "Bash", diff --git a/skillopt_sleep/multi_skill.py b/skillopt_sleep/multi_skill.py index 59aa0728..1fac42ee 100644 --- a/skillopt_sleep/multi_skill.py +++ b/skillopt_sleep/multi_skill.py @@ -16,6 +16,7 @@ from skillopt_sleep.backend import Backend, CursorBackendError from skillopt_sleep.consolidate import ConsolidationResult, consolidate from skillopt_sleep.handoff_backend import PendingCalls +from skillopt_sleep.staging import StagingError from skillopt_sleep.types import SkillGroupReport, TaskRecord CONSOLIDATED = "consolidated" @@ -123,7 +124,7 @@ def consolidate_groups( backend, list(group.tasks), group.skill, memory, **group_kwargs, ) - except (PendingCalls, CursorBackendError): + except (PendingCalls, CursorBackendError, StagingError): # These exceptions are cycle-level control flow, not isolated # evidence about one group. Swallowing them can advance/save an # incomplete handoff night or weaken Cursor's fail-closed path. diff --git a/tests/test_cli_backend_cache.py b/tests/test_cli_backend_cache.py index 2fe57589..d27af6c5 100644 --- a/tests/test_cli_backend_cache.py +++ b/tests/test_cli_backend_cache.py @@ -306,6 +306,7 @@ def test_attempt_with_tools_sets_call_local_delta(monkeypatch): from skillopt_sleep.backend import ClaudeCliBackend b = ClaudeCliBackend(model="", claude_path="claude", timeout=10) + monkeypatch.setenv("ANTHROPIC_API_KEY", "test-key") def fake_run(*args, **kwargs): return SimpleNamespace(returncode=0, stdout="ok\n", stderr="") @@ -328,6 +329,7 @@ def test_dual_backend_attempt_with_tools_sets_target_delta(monkeypatch): target = ClaudeCliBackend(model="", claude_path="claude", timeout=10) optimizer = ClaudeCliBackend(model="", claude_path="claude", timeout=10) db = DualBackend(target=target, optimizer=optimizer) + monkeypatch.setenv("ANTHROPIC_API_KEY", "test-key") def fake_run(*args, **kwargs): return SimpleNamespace(returncode=0, stdout="ok\n", stderr="") diff --git a/tests/test_sleep_engine.py b/tests/test_sleep_engine.py index bece0e97..899efd7c 100644 --- a/tests/test_sleep_engine.py +++ b/tests/test_sleep_engine.py @@ -2844,13 +2844,13 @@ def test_run_json_failure_is_one_redacted_document(self): class TestClaudeCliBackendBare(unittest.TestCase): - """Issue #68: --bare must be conditional on ANTHROPIC_API_KEY.""" + """Claude replays must fail closed when they cannot isolate user context.""" def test_bare_included_when_api_key_set(self): """With ANTHROPIC_API_KEY, --bare should appear in the command.""" from skillopt_sleep.backend import ClaudeCliBackend be = ClaudeCliBackend(claude_path="/usr/bin/false", timeout=5) - with unittest.mock.patch.dict(os.environ, {"ANTHROPIC_API_KEY": "sk-test"}): + with unittest.mock.patch.dict(os.environ, {"ANTHROPIC_API_KEY": "test-key"}): # We can't run the real CLI, but we can inspect cmd construction # by monkeypatching subprocess.run to capture the command. captured = {} @@ -2865,24 +2865,53 @@ class FakeProc: be._call("test prompt") self.assertIn("--bare", captured["cmd"]) - def test_bare_omitted_without_api_key(self): - """Without ANTHROPIC_API_KEY, --bare should NOT appear.""" + def test_oauth_without_api_key_is_refused_before_cli_invocation(self): + """OAuth cannot use bare mode, so never produce a potentially contaminated score.""" from skillopt_sleep.backend import ClaudeCliBackend + from skillopt_sleep.staging import StagingError be = ClaudeCliBackend(claude_path="/usr/bin/false", timeout=5) env = os.environ.copy() env.pop("ANTHROPIC_API_KEY", None) with unittest.mock.patch.dict(os.environ, env, clear=True): - captured = {} - def fake_run(cmd, **kwargs): - captured["cmd"] = cmd - class FakeProc: - stdout = "hello" - stderr = "" - returncode = 0 - return FakeProc() - with unittest.mock.patch("subprocess.run", side_effect=fake_run): - be._call("test prompt") - self.assertNotIn("--bare", captured["cmd"]) + with unittest.mock.patch("subprocess.run") as run: + with self.assertRaisesRegex(StagingError, "user-level instructions and hooks"): + be._call("test prompt") + run.assert_not_called() + + def test_tool_aware_call_also_uses_bare_with_api_key(self): + """The tool-aware replay path must not reintroduce ambient hooks.""" + from skillopt_sleep.backend import ClaudeCliBackend + from skillopt_sleep.types import TaskRecord + be = ClaudeCliBackend(claude_path="/usr/bin/false", timeout=5) + task = TaskRecord(id="t1", project="p", intent="test") + captured = {} + + def fake_run(cmd, **kwargs): + captured["cmd"] = cmd + class FakeProc: + stdout = "hello" + stderr = "" + returncode = 0 + return FakeProc() + + with unittest.mock.patch.dict(os.environ, {"ANTHROPIC_API_KEY": "test-key"}): + with unittest.mock.patch("skillopt_sleep.backend.subprocess.run", side_effect=fake_run): + be.attempt_with_tools(task, skill="", memory="", tools=["search"]) + self.assertIn("--bare", captured["cmd"]) + + def test_tool_aware_oauth_call_is_refused_before_cli_invocation(self): + from skillopt_sleep.backend import ClaudeCliBackend + from skillopt_sleep.staging import StagingError + from skillopt_sleep.types import TaskRecord + be = ClaudeCliBackend(claude_path="/usr/bin/false", timeout=5) + task = TaskRecord(id="t1", project="p", intent="test") + env = os.environ.copy() + env.pop("ANTHROPIC_API_KEY", None) + with unittest.mock.patch.dict(os.environ, env, clear=True): + with unittest.mock.patch("skillopt_sleep.backend.subprocess.run") as run: + with self.assertRaisesRegex(StagingError, "OAuth authentication cannot use --bare"): + be.attempt_with_tools(task, skill="", memory="", tools=["search"]) + run.assert_not_called() def test_cli_error_detected_and_logged(self): """Auth errors in CLI output should trigger a warning.""" @@ -2896,7 +2925,7 @@ class FakeProc: stderr = "" returncode = 0 return FakeProc() - with unittest.mock.patch.dict(os.environ, {}, clear=False): + with unittest.mock.patch.dict(os.environ, {"ANTHROPIC_API_KEY": "test-key"}): with unittest.mock.patch("subprocess.run", side_effect=fake_run): result = be._call("test prompt") # The error string is returned as output (backwards-compat) @@ -2912,7 +2941,8 @@ def test_spawn_failure_sets_last_call_error(self): claude_path="/nonexistent/claude-binary", timeout=3, ) - result = be._call("test prompt") + with unittest.mock.patch.dict(os.environ, {"ANTHROPIC_API_KEY": "test-key"}): + result = be._call("test prompt") self.assertEqual(result, "") self.assertIn("Claude CLI spawn failed", be.last_call_error) @@ -2930,7 +2960,8 @@ def test_attempt_tools_spawn_failure_sets_last_call_error(self): reference="ref", reference_kind="exact", tags=[], split="train", ) - resp, called = be.attempt_with_tools(task, "", "", tools=["search"]) + with unittest.mock.patch.dict(os.environ, {"ANTHROPIC_API_KEY": "test-key"}): + resp, called = be.attempt_with_tools(task, "", "", tools=["search"]) self.assertEqual(resp, "") self.assertIn("Claude CLI spawn failed", be.last_call_error) diff --git a/tests/test_sleep_multi_skill.py b/tests/test_sleep_multi_skill.py index 3e8990b4..bd406ee0 100644 --- a/tests/test_sleep_multi_skill.py +++ b/tests/test_sleep_multi_skill.py @@ -163,6 +163,19 @@ def _pending(*args, **kwargs): ) self.assertEqual(caught.exception.pending, pending) + def test_staging_refusal_propagates_instead_of_producing_a_score(self): + from skillopt_sleep.staging import StagingError + + def _refuse(*args, **kwargs): + raise StagingError("Claude replay refused: OAuth context is uncontrolled") + + with self.assertRaisesRegex(StagingError, "OAuth context is uncontrolled"): + consolidate_groups( + MockBackend(), + [SkillGroup("research-skill", "# research\n", _tasks(researcher_persona))], + consolidate_fn=_refuse, + ) + def test_fatal_cursor_error_propagates_instead_of_becoming_a_failed_row(self): from skillopt_sleep.backend import CursorBackendError diff --git a/tests/test_sleep_token_budget.py b/tests/test_sleep_token_budget.py index fae38938..9fc88175 100644 --- a/tests/test_sleep_token_budget.py +++ b/tests/test_sleep_token_budget.py @@ -543,7 +543,10 @@ def setUp(self) -> None: self._home = tempfile.TemporaryDirectory() self.addCleanup(self._proj.cleanup) self.addCleanup(self._home.cleanup) - patcher = mock.patch.dict(os.environ, {"SKILLOPT_SLEEP_WORKERS": "1"}) + patcher = mock.patch.dict(os.environ, { + "SKILLOPT_SLEEP_WORKERS": "1", + "ANTHROPIC_API_KEY": "test-key", + }) patcher.start() self.addCleanup(patcher.stop) @@ -633,7 +636,10 @@ def spent() -> bool: # The budget runs out once the skill step's reflect call started. return ("train", "text") in process.calls - with mock.patch.dict(os.environ, {"SKILLOPT_SLEEP_WORKERS": "1"}), \ + with mock.patch.dict(os.environ, { + "SKILLOPT_SLEEP_WORKERS": "1", + "ANTHROPIC_API_KEY": "test-key", + }), \ mock.patch("skillopt_sleep.backend.subprocess.run", side_effect=process): result = consolidate( backend, tasks, "# skill\n", "", budget_exhausted=spent