Skip to content

Commit f878680

Browse files
node9aiclaude
andcommitted
docs+test: move ⚠️ warnings to top of evaluate docstring; harden malformed-response test
- Move both ⚠️ warnings (low-level primitive, fail-open default) to the top of the evaluate() docstring so developers scanning quickly see them first - Pin test_saas_malformed_response_raises to ActionDeniedException only (missing approved+pending → ActionDeniedException is the exact code path, not a coincidental raise — documents the guarantee) - Case-insensitive Bearer header lookup in bearer token test (urllib capitalizes header keys, making case-sensitive get() fragile across Python versions) - Update stale comment in test_evaluate_importable_from_public_api to reference the renamed test and correct behavior (raise, not warn) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent d6684f6 commit f878680

2 files changed

Lines changed: 23 additions & 15 deletions

File tree

‎node9/_client.py‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,15 @@ def _evaluate_cloud(tool_name: str, args: dict[str, Any], run_id: str = "") -> N
267267

268268
def evaluate(tool_name: str, args: dict[str, Any], *, run_id: str = "") -> None:
269269
"""
270+
⚠️ evaluate() is a low-level primitive. Unlike @protect it does NOT run DLP
271+
scanning or inject agent context (run_id, agent_name) automatically. Use
272+
@protect for full protection; use evaluate() only when decorator composition
273+
is not possible (e.g. dynamic tool dispatch in custom frameworks).
274+
275+
⚠️ Default is fail-open: when neither daemon nor API key is available,
276+
actions are auto-approved (offline audit mode). The exception is
277+
policy == "require_approval" which raises DaemonNotFoundError (fail-closed).
278+
270279
Sends the action to node9 for audit / approval.
271280
272281
Routing (first match wins):
@@ -277,9 +286,7 @@ def evaluate(tool_name: str, args: dict[str, Any], *, run_id: str = "") -> None:
277286
278287
Fail behaviour:
279288
- Daemon unreachable at call time → offline mode (fail-open, auto-approve)
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.
289+
Exception: policy == "require_approval" → DaemonNotFoundError (fail-closed).
283290
- Daemon dies mid-request → DaemonNotFoundError or ActionDeniedException
284291
(fail-closed: no auto-approve path exists once a request_id is acquired)
285292
- Daemon timeout / connection closed → ActionDeniedException (fail-closed)
@@ -288,11 +295,6 @@ def evaluate(tool_name: str, args: dict[str, Any], *, run_id: str = "") -> None:
288295
Timeouts: see _CHECK_TIMEOUT (initial connection) and _WAIT_TIMEOUT
289296
(human decision wait) module constants for current values.
290297
291-
⚠️ evaluate() is a low-level primitive. Unlike @protect it does NOT run DLP
292-
scanning or inject agent context (run_id, agent_name) automatically. Use
293-
@protect for full protection; use evaluate() only when decorator composition
294-
is not possible (e.g. dynamic tool dispatch in custom frameworks).
295-
296298
Raises ActionDeniedException if the action is denied.
297299
"""
298300
if _SKIP:

‎tests/test_client.py‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@ def test_evaluate_importable_from_public_api():
1616
separately to verify the export is intentional and discoverable by linters
1717
and import-* consumers.
1818
19-
Fail-open / offline-mode coverage lives in TestOfflineMode (4 tests), which
20-
verifies auto-approve behavior, audit log writes, and the require_approval
21-
RuntimeWarning path.
19+
Fail-open / offline-mode coverage lives in TestOfflineMode, which verifies
20+
auto-approve behavior, audit log writes, and require_approval fail-closed path
21+
(raises DaemonNotFoundError — see test_offline_with_require_approval_policy_raises).
2222
"""
2323
import node9
2424
from node9 import evaluate as pub_evaluate
@@ -380,7 +380,10 @@ def capturing_urlopen(req, timeout):
380380
mock_check.assert_not_called()
381381

382382
assert captured_headers, "urlopen was never called"
383-
auth = captured_headers[0].get("Authorization", "")
383+
# urllib capitalizes header keys (e.g. "authorization" → "Authorization"),
384+
# so use case-insensitive lookup to avoid false negatives.
385+
headers_lower = {k.lower(): v for k, v in captured_headers[0].items()}
386+
auth = headers_lower.get("authorization", "")
384387
assert auth == "Bearer test-key-abc", f"Expected Bearer token, got: {auth!r}"
385388

386389
def test_saas_denial_raises_action_denied_exception(self, monkeypatch):
@@ -395,14 +398,17 @@ def test_saas_denial_raises_action_denied_exception(self, monkeypatch):
395398
assert "Blocked by policy" in str(exc.value)
396399

397400
def test_saas_malformed_response_raises(self, monkeypatch):
398-
"""SaaS response missing 'approved' key must not silently auto-approve."""
401+
"""SaaS response missing 'approved' key must raise ActionDeniedException, not auto-approve.
402+
403+
_evaluate_cloud logic: approved missing → falsy; pending missing → falsy →
404+
raises ActionDeniedException("Denied by Node9 policy"). No fall-through to offline mode.
405+
"""
399406
monkeypatch.setenv("NODE9_API_KEY", "test-key")
400407
monkeypatch.setenv("NODE9_API_URL", "https://api.node9.ai/api/v1/intercept")
401408

402-
# Missing both 'approved' and 'pending' — should raise, not auto-approve
403409
malformed = _make_response({"status": "unknown"})
404410
with patch("urllib.request.urlopen", return_value=malformed):
405-
with pytest.raises((ActionDeniedException, RuntimeError)):
411+
with pytest.raises(ActionDeniedException):
406412
evaluate("bash", {"command": "ls"})
407413

408414
def test_saas_run_id_defaults_to_empty_string(self, monkeypatch):

0 commit comments

Comments
 (0)