Skip to content

Commit d6684f6

Browse files
node9aiclaude
andcommitted
fix(security): require_approval + offline now raises DaemonNotFoundError (fail-closed)
Previously, require_approval policy with no daemon or API key degraded silently to auto-approve with a suppressible RuntimeWarning. This contradicts the policy name and creates a false sense of security — an operator who configured require_approval would not expect their agents to self-approve when governance infra is unreachable. Change: _offline_audit raises DaemonNotFoundError when policy == "require_approval". Operators must fix connectivity (start daemon or set NODE9_API_KEY) rather than silently proceeding without governance. Also: - Update docstring to reflect fail-closed behavior for require_approval - Remove misleading comment from bearer token test - Add test_saas_malformed_response_raises: missing 'approved' key must not auto-approve - Add test_saas_run_id_defaults_to_empty_string: run_id absent → "" in payload Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent f44cff1 commit d6684f6

2 files changed

Lines changed: 45 additions & 21 deletions

File tree

‎node9/_client.py‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -129,14 +129,10 @@ def _offline_audit(tool_name: str, args: dict[str, Any], run_id: str) -> None:
129129
import datetime
130130
_, policy = _config.get()
131131
if policy == "require_approval":
132-
import warnings
133-
warnings.warn(
134-
f"[Node9] Governance degraded to offline/auto-approve for '{tool_name}' — "
135-
"policy is 'require_approval' but no daemon or API key is available. "
136-
"Start the node9 daemon or set NODE9_API_KEY to enforce approvals.",
137-
RuntimeWarning,
138-
stacklevel=4,
139-
)
132+
# Fail-closed: require_approval means governance must be enforced.
133+
# Auto-approving silently would contradict the policy name and create
134+
# a false sense of security. Raise so operators see the misconfiguration.
135+
raise DaemonNotFoundError(DAEMON_PORT)
140136
audit_dir = os.path.join(os.path.expanduser("~"), ".node9")
141137
os.makedirs(audit_dir, exist_ok=True)
142138
audit_path = os.path.join(audit_dir, "audit.log")
@@ -281,8 +277,9 @@ def evaluate(tool_name: str, args: dict[str, Any], *, run_id: str = "") -> None:
281277
282278
Fail behaviour:
283279
- Daemon unreachable at call time → offline mode (fail-open, auto-approve)
284-
• If policy == "require_approval" a RuntimeWarning is emitted so the
285-
degradation is visible in logs even when nothing blocks.
280+
⚠️ Exception: policy == "require_approval" → DaemonNotFoundError (fail-closed).
281+
Auto-approving when approval is explicitly required would be a silent
282+
security bypass; operators must fix the connectivity issue instead.
286283
- Daemon dies mid-request → DaemonNotFoundError or ActionDeniedException
287284
(fail-closed: no auto-approve path exists once a request_id is acquired)
288285
- Daemon timeout / connection closed → ActionDeniedException (fail-closed)

‎tests/test_client.py‎

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -315,22 +315,20 @@ def test_offline_mode_auto_approves(self, monkeypatch, tmp_path):
315315
result = evaluate("bash", {"command": "ls"})
316316
assert result is None
317317

318-
def test_offline_with_require_approval_policy_warns(self, monkeypatch, tmp_path):
319-
"""Offline auto-approve must warn loudly when policy is require_approval."""
320-
import warnings
318+
def test_offline_with_require_approval_policy_raises(self, monkeypatch, tmp_path):
319+
"""require_approval + no daemon/API key must raise DaemonNotFoundError (fail-closed).
320+
321+
Auto-approving silently would contradict the policy name — raise so operators
322+
see the misconfiguration rather than getting a false sense of security.
323+
"""
321324
import node9._config as cfg
322325
monkeypatch.delenv("NODE9_API_KEY", raising=False)
323326
monkeypatch.delenv("NODE9_AUTO_START", raising=False)
324327
monkeypatch.setenv("HOME", str(tmp_path))
325328
monkeypatch.setattr(cfg, "AGENT_POLICY", "require_approval")
326329
with patch("node9._client._daemon_reachable", return_value=False):
327-
with warnings.catch_warnings(record=True) as caught:
328-
warnings.simplefilter("always")
330+
with pytest.raises(DaemonNotFoundError):
329331
evaluate("deploy", {"target": "prod"})
330-
assert any(
331-
issubclass(w.category, RuntimeWarning) and "require_approval" in str(w.message)
332-
for w in caught
333-
), "Expected RuntimeWarning for offline degradation under require_approval policy"
334332

335333

336334
class TestSaaSRoute:
@@ -379,7 +377,6 @@ def capturing_urlopen(req, timeout):
379377
with patch("urllib.request.urlopen", side_effect=capturing_urlopen):
380378
with patch("node9._client._daemon_reachable") as mock_check:
381379
evaluate("bash", {"command": "ls"})
382-
# Assert inside the with block so mock is still active
383380
mock_check.assert_not_called()
384381

385382
assert captured_headers, "urlopen was never called"
@@ -395,7 +392,37 @@ def test_saas_denial_raises_action_denied_exception(self, monkeypatch):
395392
with patch("urllib.request.urlopen", return_value=deny_resp):
396393
with pytest.raises(ActionDeniedException) as exc:
397394
evaluate("bash", {"command": "rm -rf /"})
398-
assert "Blocked by policy" in exc.value.reason
395+
assert "Blocked by policy" in str(exc.value)
396+
397+
def test_saas_malformed_response_raises(self, monkeypatch):
398+
"""SaaS response missing 'approved' key must not silently auto-approve."""
399+
monkeypatch.setenv("NODE9_API_KEY", "test-key")
400+
monkeypatch.setenv("NODE9_API_URL", "https://api.node9.ai/api/v1/intercept")
401+
402+
# Missing both 'approved' and 'pending' — should raise, not auto-approve
403+
malformed = _make_response({"status": "unknown"})
404+
with patch("urllib.request.urlopen", return_value=malformed):
405+
with pytest.raises((ActionDeniedException, RuntimeError)):
406+
evaluate("bash", {"command": "ls"})
407+
408+
def test_saas_run_id_defaults_to_empty_string(self, monkeypatch):
409+
"""run_id omitted from call → empty string sent in SaaS payload (not None / missing)."""
410+
monkeypatch.setenv("NODE9_API_KEY", "test-key")
411+
monkeypatch.setenv("NODE9_API_URL", "https://api.node9.ai/api/v1/intercept")
412+
413+
sent: list[dict] = []
414+
approve_resp = _make_response({"approved": True})
415+
416+
def capturing_urlopen(req, timeout):
417+
if hasattr(req, "data") and req.data:
418+
sent.append(json.loads(req.data))
419+
return approve_resp
420+
421+
with patch("urllib.request.urlopen", side_effect=capturing_urlopen):
422+
evaluate("bash", {"command": "ls"}) # no run_id argument
423+
424+
assert sent, "No request sent"
425+
assert sent[0].get("runId") == "", f"Expected empty string runId, got: {sent[0].get('runId')!r}"
399426

400427
def test_saas_run_id_forwarded(self, monkeypatch):
401428
"""run_id is included in the SaaS request payload."""

0 commit comments

Comments
 (0)