From ebd3c50df7091fdae13377cdee1d2456c74db567 Mon Sep 17 00:00:00 2001 From: node9 Date: Wed, 8 Apr 2026 12:47:14 +0300 Subject: [PATCH 01/11] fix: skip PR creation when dev has no new commits vs main Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/auto-pr.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/auto-pr.yml b/.github/workflows/auto-pr.yml index 30de97b..de35fff 100644 --- a/.github/workflows/auto-pr.yml +++ b/.github/workflows/auto-pr.yml @@ -26,6 +26,12 @@ jobs: > - `feat:` to publish a Minor release (0.X.0) > If it starts with `chore:`, no PyPI package will be published! run: | + git fetch origin main + COMMITS=$(git log origin/main..origin/dev --oneline | wc -l) + if [ "$COMMITS" -eq 0 ]; then + echo "No commits between dev and main — nothing to release, skipping PR." + exit 0 + fi PR=$(gh pr list --base main --head dev --state open --json number -q '.[0].number') if [ -z "$PR" ]; then gh pr create \ From 6ecd581af94db5aa1f0fa2a97ddc21a5300cb97f Mon Sep 17 00:00:00 2001 From: node9 Date: Wed, 8 Apr 2026 15:36:49 +0300 Subject: [PATCH 02/11] feat: export evaluate() as public API Co-Authored-By: Claude Sonnet 4.6 --- node9/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/node9/__init__.py b/node9/__init__.py index e872f0d..7be053d 100644 --- a/node9/__init__.py +++ b/node9/__init__.py @@ -6,6 +6,7 @@ from ._exceptions import ActionDeniedException, DaemonNotFoundError from ._dlp import dlp_scan, safe_path from ._agent import Node9Agent, tool, internal +from ._client import evaluate from . import _config @@ -41,6 +42,8 @@ def configure(*, agent_name: str = "", policy: str = "") -> None: # .build_tools_openai() — OpenAI function format # .dispatch(name, input) — route LLM tool call to @tool method # .new_session() — fresh run_id for server/multi-session deployments + # Low-level governance + "evaluate", # manual evaluate() without @protect decorator # DLP utilities "dlp_scan", "safe_path", From b9fcb17a0e9f683c638c77e3f5a6def2e64104c1 Mon Sep 17 00:00:00 2001 From: node9 Date: Wed, 8 Apr 2026 16:28:51 +0300 Subject: [PATCH 03/11] docs(evaluate): document fail behaviour, auth, and timeouts; add public API import test Addresses code review feedback: - Expand evaluate() docstring to explicitly cover fail-open vs fail-closed per routing path, authentication model, and timeout values - Add test_evaluate_importable_from_public_api to verify __all__ export Co-Authored-By: Claude Sonnet 4.6 --- node9/_client.py | 21 +++++++++++++++++---- tests/test_client.py | 6 ++++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/node9/_client.py b/node9/_client.py index 92f7dc6..fab7420 100644 --- a/node9/_client.py +++ b/node9/_client.py @@ -271,12 +271,25 @@ def _evaluate_cloud(tool_name: str, args: dict[str, Any], run_id: str = "") -> N def evaluate(tool_name: str, args: dict[str, Any], *, run_id: str = "") -> None: """ - Sends the action to node9 for audit / approval. Routing: - NODE9_SKIP=1 → no-op (unsafe bypass for testing) - NODE9_API_KEY set → node9 SaaS - daemon reachable → local proxy + Sends the action to node9 for audit / approval. + + Routing (first match wins): + NODE9_SKIP=1 → no-op (unsafe bypass — testing only, emits a warning) + NODE9_API_KEY set → node9 SaaS (HTTPS + Bearer token auth) + daemon reachable → local proxy on 127.0.0.1 (no auth — loopback only) neither → offline audit log (auto-approve, never blocks) + Fail behaviour: + - Daemon timeout / connection closed → ActionDeniedException (fail-closed) + - Daemon unreachable at call time → offline mode (fail-open, auto-approve) + • If policy == "require_approval" a RuntimeWarning is emitted so the + degradation is visible in logs even when nothing blocks. + - SaaS HTTP error or timeout → RuntimeError (propagates to caller) + + Timeouts: + - Initial connection: _CHECK_TIMEOUT (5 s) + - Waiting for human decision: _WAIT_TIMEOUT (65 s) — daemon auto-denies ~55 s + Raises ActionDeniedException if the action is denied. """ if _SKIP: diff --git a/tests/test_client.py b/tests/test_client.py index 0b48e7b..e53c4e8 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -8,6 +8,12 @@ from node9._client import evaluate +def test_evaluate_importable_from_public_api(): + """evaluate must be importable from the top-level node9 package (via __all__).""" + from node9 import evaluate as pub_evaluate # noqa: F401 + assert callable(pub_evaluate) + + def _make_response(data: dict): """Create a mock urllib response.""" From 40d12bb8b0d92266b180c1fa9fe6f729c2d3eb61 Mon Sep 17 00:00:00 2001 From: node9 Date: Wed, 8 Apr 2026 16:31:41 +0300 Subject: [PATCH 04/11] test(evaluate): assert __all__ membership directly, document existing offline coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous test verified importability but not __all__ inclusion — from node9 import evaluate resolves via __init__.py regardless of __all__. Adding the explicit assert "evaluate" in node9.__all__ closes that gap. Also adds a docstring note pointing reviewers to TestOfflineMode (which already covers fail-open, audit log, and require_approval warning paths). Co-Authored-By: Claude Sonnet 4.6 --- tests/test_client.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/tests/test_client.py b/tests/test_client.py index e53c4e8..be0e58a 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -9,10 +9,21 @@ def test_evaluate_importable_from_public_api(): - """evaluate must be importable from the top-level node9 package (via __all__).""" - from node9 import evaluate as pub_evaluate # noqa: F401 + """evaluate is in __all__ and importable from the top-level node9 package. + + Note: from node9 import evaluate would succeed even with a broken __all__ + (because __init__.py imports it directly), so we assert __all__ membership + separately to verify the export is intentional and discoverable by linters + and import-* consumers. + + Fail-open / offline-mode coverage lives in TestOfflineMode (4 tests), which + verifies auto-approve behavior, audit log writes, and the require_approval + RuntimeWarning path. + """ + import node9 + from node9 import evaluate as pub_evaluate assert callable(pub_evaluate) - + assert "evaluate" in node9.__all__ def _make_response(data: dict): From 2fab1c707dbd30c69042987ff6a770a51acc999e Mon Sep 17 00:00:00 2001 From: node9 Date: Wed, 8 Apr 2026 16:33:56 +0300 Subject: [PATCH 05/11] ci(auto-pr): clarify exit 0 intent with inline comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The exit 0 guard is correct — gh pr create is in the same run: block so exiting mid-script prevents it from running. Add a comment so reviewers don't question this pattern. Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/auto-pr.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/auto-pr.yml b/.github/workflows/auto-pr.yml index de35fff..4c25816 100644 --- a/.github/workflows/auto-pr.yml +++ b/.github/workflows/auto-pr.yml @@ -30,6 +30,9 @@ jobs: COMMITS=$(git log origin/main..origin/dev --oneline | wc -l) if [ "$COMMITS" -eq 0 ]; then echo "No commits between dev and main — nothing to release, skipping PR." + # exit 0 here is correct: the gh pr create below is in the same run: block, + # so exiting the shell mid-script prevents it from running. There are no + # downstream steps in this job that would be skipped incorrectly. exit 0 fi PR=$(gh pr list --base main --head dev --state open --json number -q '.[0].number') From 2bd6400e7631456fd0f2c448e3e61d5815b04647 Mon Sep 17 00:00:00 2001 From: node9 Date: Wed, 8 Apr 2026 16:38:26 +0300 Subject: [PATCH 06/11] fix(evaluate): document @protect bypass, clarify TOCTOU safety; add SaaS path tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Docstring: add ⚠️ callout that evaluate() skips DLP and agent context injection — callers should prefer @protect unless decorator composition is impossible - Docstring: clarify daemon-dies-mid-request is fail-closed (DaemonNotFoundError or ActionDeniedException), not a TOCTOU auto-approve path - Docstring: reference _CHECK_TIMEOUT / _WAIT_TIMEOUT constants by name - Tests: add TestSaaSRoute with 3 cases covering HTTP error → RuntimeError, URL error → RuntimeError, and API key routing bypasses daemon check Co-Authored-By: Claude Sonnet 4.6 --- node9/_client.py | 15 +++++++++++---- tests/test_client.py | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/node9/_client.py b/node9/_client.py index fab7420..01415c6 100644 --- a/node9/_client.py +++ b/node9/_client.py @@ -280,15 +280,22 @@ def evaluate(tool_name: str, args: dict[str, Any], *, run_id: str = "") -> None: neither → offline audit log (auto-approve, never blocks) Fail behaviour: - - Daemon timeout / connection closed → ActionDeniedException (fail-closed) - Daemon unreachable at call time → offline mode (fail-open, auto-approve) • If policy == "require_approval" a RuntimeWarning is emitted so the degradation is visible in logs even when nothing blocks. + - Daemon dies mid-request → DaemonNotFoundError or ActionDeniedException + (fail-closed: no auto-approve path exists once a request_id is acquired) + - Daemon timeout / connection closed → ActionDeniedException (fail-closed) - SaaS HTTP error or timeout → RuntimeError (propagates to caller) - Timeouts: - - Initial connection: _CHECK_TIMEOUT (5 s) - - Waiting for human decision: _WAIT_TIMEOUT (65 s) — daemon auto-denies ~55 s + Timeouts (see _CHECK_TIMEOUT / _WAIT_TIMEOUT module constants): + - Initial connection: 5 s + - Waiting for human decision: 65 s — daemon auto-denies at ~55 s + + ⚠️ evaluate() is a low-level primitive. Unlike @protect it does NOT run DLP + scanning or inject agent context (run_id, agent_name) automatically. Use + @protect for full protection; use evaluate() only when decorator composition + is not possible (e.g. dynamic tool dispatch in custom frameworks). Raises ActionDeniedException if the action is denied. """ diff --git a/tests/test_client.py b/tests/test_client.py index be0e58a..39c6801 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -333,6 +333,49 @@ def test_offline_with_require_approval_policy_warns(self, monkeypatch, tmp_path) ), "Expected RuntimeWarning for offline degradation under require_approval policy" +class TestSaaSRoute: + """Tests for the NODE9_API_KEY → SaaS routing path.""" + + def test_saas_http_error_raises_runtime_error(self, monkeypatch, tmp_path): + """SaaS HTTP errors (e.g. 401, 500) must propagate as RuntimeError, not auto-approve.""" + import urllib.error + monkeypatch.setenv("NODE9_API_KEY", "test-key") + monkeypatch.setenv("NODE9_API_URL", "https://api.node9.ai/api/v1/intercept") + + http_error = urllib.error.HTTPError( + url="https://api.node9.ai/api/v1/intercept", + code=401, + msg="Unauthorized", + hdrs=None, # type: ignore[arg-type] + fp=None, + ) + with patch("urllib.request.urlopen", side_effect=http_error): + with pytest.raises(RuntimeError, match="401"): + evaluate("bash", {"command": "ls"}) + + def test_saas_url_error_raises_runtime_error(self, monkeypatch): + """SaaS connectivity failure must propagate as RuntimeError, not auto-approve.""" + import urllib.error + monkeypatch.setenv("NODE9_API_KEY", "test-key") + monkeypatch.setenv("NODE9_API_URL", "https://api.node9.ai/api/v1/intercept") + + with patch("urllib.request.urlopen", side_effect=urllib.error.URLError("timeout")): + with pytest.raises(RuntimeError, match="Failed to reach node9 SaaS"): + evaluate("bash", {"command": "ls"}) + + def test_saas_route_taken_when_api_key_set(self, monkeypatch): + """When NODE9_API_KEY is set the SaaS path is taken regardless of daemon state.""" + monkeypatch.setenv("NODE9_API_KEY", "test-key") + monkeypatch.setenv("NODE9_API_URL", "https://api.node9.ai/api/v1/intercept") + + approve_resp = _make_response({"approved": True}) + with patch("urllib.request.urlopen", return_value=approve_resp): + with patch("node9._client._daemon_reachable") as mock_check: + evaluate("bash", {"command": "ls"}) + # daemon reachability must NOT be checked when API key is present + mock_check.assert_not_called() + + class TestConfigure: def test_configure_sets_agent_name(self): from node9 import configure From d5357ab3c4f58a7cd120f185dfe8400b6af05ac0 Mon Sep 17 00:00:00 2001 From: node9 Date: Wed, 8 Apr 2026 16:41:04 +0300 Subject: [PATCH 07/11] test(saas): fix HTTPError mock, assert Bearer token, add run_id test; trim docstring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix urllib.error.HTTPError mock: hdrs=None → http.client.HTTPMessage() (prevents AttributeError on Python versions that read headers from error) - test_saas_route_taken_and_bearer_token_sent: assert Authorization header contains the correct Bearer token (was only checking routing, not auth) - test_saas_run_id_forwarded: verify run_id reaches the SaaS payload - docstring: remove hardcoded 5s/65s values, reference constants by name only Co-Authored-By: Claude Sonnet 4.6 --- node9/_client.py | 5 ++--- tests/test_client.py | 43 ++++++++++++++++++++++++++++++++++++------- 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/node9/_client.py b/node9/_client.py index 01415c6..81f61e4 100644 --- a/node9/_client.py +++ b/node9/_client.py @@ -288,9 +288,8 @@ def evaluate(tool_name: str, args: dict[str, Any], *, run_id: str = "") -> None: - Daemon timeout / connection closed → ActionDeniedException (fail-closed) - SaaS HTTP error or timeout → RuntimeError (propagates to caller) - Timeouts (see _CHECK_TIMEOUT / _WAIT_TIMEOUT module constants): - - Initial connection: 5 s - - Waiting for human decision: 65 s — daemon auto-denies at ~55 s + Timeouts: see _CHECK_TIMEOUT (initial connection) and _WAIT_TIMEOUT + (human decision wait) module constants for current values. ⚠️ evaluate() is a low-level primitive. Unlike @protect it does NOT run DLP scanning or inject agent context (run_id, agent_name) automatically. Use diff --git a/tests/test_client.py b/tests/test_client.py index 39c6801..be00c97 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -336,8 +336,9 @@ def test_offline_with_require_approval_policy_warns(self, monkeypatch, tmp_path) class TestSaaSRoute: """Tests for the NODE9_API_KEY → SaaS routing path.""" - def test_saas_http_error_raises_runtime_error(self, monkeypatch, tmp_path): + def test_saas_http_error_raises_runtime_error(self, monkeypatch): """SaaS HTTP errors (e.g. 401, 500) must propagate as RuntimeError, not auto-approve.""" + import http.client import urllib.error monkeypatch.setenv("NODE9_API_KEY", "test-key") monkeypatch.setenv("NODE9_API_URL", "https://api.node9.ai/api/v1/intercept") @@ -346,7 +347,7 @@ def test_saas_http_error_raises_runtime_error(self, monkeypatch, tmp_path): url="https://api.node9.ai/api/v1/intercept", code=401, msg="Unauthorized", - hdrs=None, # type: ignore[arg-type] + hdrs=http.client.HTTPMessage(), fp=None, ) with patch("urllib.request.urlopen", side_effect=http_error): @@ -363,17 +364,45 @@ def test_saas_url_error_raises_runtime_error(self, monkeypatch): with pytest.raises(RuntimeError, match="Failed to reach node9 SaaS"): evaluate("bash", {"command": "ls"}) - def test_saas_route_taken_when_api_key_set(self, monkeypatch): - """When NODE9_API_KEY is set the SaaS path is taken regardless of daemon state.""" - monkeypatch.setenv("NODE9_API_KEY", "test-key") + def test_saas_route_taken_and_bearer_token_sent(self, monkeypatch): + """When NODE9_API_KEY is set: SaaS path is taken and Bearer token is in the request.""" + monkeypatch.setenv("NODE9_API_KEY", "test-key-abc") monkeypatch.setenv("NODE9_API_URL", "https://api.node9.ai/api/v1/intercept") + captured_headers: list[dict] = [] approve_resp = _make_response({"approved": True}) - with patch("urllib.request.urlopen", return_value=approve_resp): + + def capturing_urlopen(req, timeout): + captured_headers.append(dict(req.headers)) + return approve_resp + + with patch("urllib.request.urlopen", side_effect=capturing_urlopen): with patch("node9._client._daemon_reachable") as mock_check: evaluate("bash", {"command": "ls"}) - # daemon reachability must NOT be checked when API key is present + mock_check.assert_not_called() + assert captured_headers, "urlopen was never called" + auth = captured_headers[0].get("Authorization", "") + assert auth == "Bearer test-key-abc", f"Expected Bearer token, got: {auth!r}" + + def test_saas_run_id_forwarded(self, monkeypatch): + """run_id is included in the SaaS request payload.""" + monkeypatch.setenv("NODE9_API_KEY", "test-key") + monkeypatch.setenv("NODE9_API_URL", "https://api.node9.ai/api/v1/intercept") + + sent: list[dict] = [] + approve_resp = _make_response({"approved": True}) + + def capturing_urlopen(req, timeout): + if hasattr(req, "data") and req.data: + sent.append(json.loads(req.data)) + return approve_resp + + with patch("urllib.request.urlopen", side_effect=capturing_urlopen): + evaluate("bash", {"command": "ls"}, run_id="saas-run-xyz") + + assert sent, "No request was sent" + assert sent[0].get("runId") == "saas-run-xyz" class TestConfigure: From f44cff124a8051131b3cddbe49cd11c7aa23c370 Mon Sep 17 00:00:00 2001 From: node9 Date: Wed, 8 Apr 2026 16:43:43 +0300 Subject: [PATCH 08/11] test(saas): add denial path test; move assert_not_called inside with block - test_saas_denial_raises_action_denied_exception: verify approved=False raises ActionDeniedException with correct reason (core SaaS security path) - Move mock_check.assert_not_called() inside the patch context manager for explicit correctness (mock records during the with block) Co-Authored-By: Claude Sonnet 4.6 --- tests/test_client.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/test_client.py b/tests/test_client.py index be00c97..85fa324 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -379,12 +379,24 @@ def capturing_urlopen(req, timeout): with patch("urllib.request.urlopen", side_effect=capturing_urlopen): with patch("node9._client._daemon_reachable") as mock_check: evaluate("bash", {"command": "ls"}) + # Assert inside the with block so mock is still active + mock_check.assert_not_called() - mock_check.assert_not_called() assert captured_headers, "urlopen was never called" auth = captured_headers[0].get("Authorization", "") assert auth == "Bearer test-key-abc", f"Expected Bearer token, got: {auth!r}" + def test_saas_denial_raises_action_denied_exception(self, monkeypatch): + """SaaS returning approved=False must raise ActionDeniedException, not auto-approve.""" + monkeypatch.setenv("NODE9_API_KEY", "test-key") + monkeypatch.setenv("NODE9_API_URL", "https://api.node9.ai/api/v1/intercept") + + deny_resp = _make_response({"approved": False, "pending": False, "reason": "Blocked by policy"}) + with patch("urllib.request.urlopen", return_value=deny_resp): + with pytest.raises(ActionDeniedException) as exc: + evaluate("bash", {"command": "rm -rf /"}) + assert "Blocked by policy" in exc.value.reason + def test_saas_run_id_forwarded(self, monkeypatch): """run_id is included in the SaaS request payload.""" monkeypatch.setenv("NODE9_API_KEY", "test-key") From d6684f6ff6f55def1ca01e728b6f7e41c38dbe78 Mon Sep 17 00:00:00 2001 From: node9 Date: Wed, 8 Apr 2026 16:47:31 +0300 Subject: [PATCH 09/11] fix(security): require_approval + offline now raises DaemonNotFoundError (fail-closed) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- node9/_client.py | 17 +++++++-------- tests/test_client.py | 49 ++++++++++++++++++++++++++++++++++---------- 2 files changed, 45 insertions(+), 21 deletions(-) diff --git a/node9/_client.py b/node9/_client.py index 81f61e4..0c634fe 100644 --- a/node9/_client.py +++ b/node9/_client.py @@ -129,14 +129,10 @@ def _offline_audit(tool_name: str, args: dict[str, Any], run_id: str) -> None: import datetime _, policy = _config.get() if policy == "require_approval": - import warnings - warnings.warn( - f"[Node9] Governance degraded to offline/auto-approve for '{tool_name}' — " - "policy is 'require_approval' but no daemon or API key is available. " - "Start the node9 daemon or set NODE9_API_KEY to enforce approvals.", - RuntimeWarning, - stacklevel=4, - ) + # Fail-closed: require_approval means governance must be enforced. + # Auto-approving silently would contradict the policy name and create + # a false sense of security. Raise so operators see the misconfiguration. + raise DaemonNotFoundError(DAEMON_PORT) audit_dir = os.path.join(os.path.expanduser("~"), ".node9") os.makedirs(audit_dir, exist_ok=True) 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: Fail behaviour: - Daemon unreachable at call time → offline mode (fail-open, auto-approve) - • If policy == "require_approval" a RuntimeWarning is emitted so the - degradation is visible in logs even when nothing blocks. + ⚠️ Exception: policy == "require_approval" → DaemonNotFoundError (fail-closed). + Auto-approving when approval is explicitly required would be a silent + security bypass; operators must fix the connectivity issue instead. - Daemon dies mid-request → DaemonNotFoundError or ActionDeniedException (fail-closed: no auto-approve path exists once a request_id is acquired) - Daemon timeout / connection closed → ActionDeniedException (fail-closed) diff --git a/tests/test_client.py b/tests/test_client.py index 85fa324..d24c545 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -315,22 +315,20 @@ def test_offline_mode_auto_approves(self, monkeypatch, tmp_path): result = evaluate("bash", {"command": "ls"}) assert result is None - def test_offline_with_require_approval_policy_warns(self, monkeypatch, tmp_path): - """Offline auto-approve must warn loudly when policy is require_approval.""" - import warnings + def test_offline_with_require_approval_policy_raises(self, monkeypatch, tmp_path): + """require_approval + no daemon/API key must raise DaemonNotFoundError (fail-closed). + + Auto-approving silently would contradict the policy name — raise so operators + see the misconfiguration rather than getting a false sense of security. + """ import node9._config as cfg monkeypatch.delenv("NODE9_API_KEY", raising=False) monkeypatch.delenv("NODE9_AUTO_START", raising=False) monkeypatch.setenv("HOME", str(tmp_path)) monkeypatch.setattr(cfg, "AGENT_POLICY", "require_approval") with patch("node9._client._daemon_reachable", return_value=False): - with warnings.catch_warnings(record=True) as caught: - warnings.simplefilter("always") + with pytest.raises(DaemonNotFoundError): evaluate("deploy", {"target": "prod"}) - assert any( - issubclass(w.category, RuntimeWarning) and "require_approval" in str(w.message) - for w in caught - ), "Expected RuntimeWarning for offline degradation under require_approval policy" class TestSaaSRoute: @@ -379,7 +377,6 @@ def capturing_urlopen(req, timeout): with patch("urllib.request.urlopen", side_effect=capturing_urlopen): with patch("node9._client._daemon_reachable") as mock_check: evaluate("bash", {"command": "ls"}) - # Assert inside the with block so mock is still active mock_check.assert_not_called() assert captured_headers, "urlopen was never called" @@ -395,7 +392,37 @@ def test_saas_denial_raises_action_denied_exception(self, monkeypatch): with patch("urllib.request.urlopen", return_value=deny_resp): with pytest.raises(ActionDeniedException) as exc: evaluate("bash", {"command": "rm -rf /"}) - assert "Blocked by policy" in exc.value.reason + assert "Blocked by policy" in str(exc.value) + + def test_saas_malformed_response_raises(self, monkeypatch): + """SaaS response missing 'approved' key must not silently auto-approve.""" + monkeypatch.setenv("NODE9_API_KEY", "test-key") + monkeypatch.setenv("NODE9_API_URL", "https://api.node9.ai/api/v1/intercept") + + # Missing both 'approved' and 'pending' — should raise, not auto-approve + malformed = _make_response({"status": "unknown"}) + with patch("urllib.request.urlopen", return_value=malformed): + with pytest.raises((ActionDeniedException, RuntimeError)): + evaluate("bash", {"command": "ls"}) + + def test_saas_run_id_defaults_to_empty_string(self, monkeypatch): + """run_id omitted from call → empty string sent in SaaS payload (not None / missing).""" + monkeypatch.setenv("NODE9_API_KEY", "test-key") + monkeypatch.setenv("NODE9_API_URL", "https://api.node9.ai/api/v1/intercept") + + sent: list[dict] = [] + approve_resp = _make_response({"approved": True}) + + def capturing_urlopen(req, timeout): + if hasattr(req, "data") and req.data: + sent.append(json.loads(req.data)) + return approve_resp + + with patch("urllib.request.urlopen", side_effect=capturing_urlopen): + evaluate("bash", {"command": "ls"}) # no run_id argument + + assert sent, "No request sent" + assert sent[0].get("runId") == "", f"Expected empty string runId, got: {sent[0].get('runId')!r}" def test_saas_run_id_forwarded(self, monkeypatch): """run_id is included in the SaaS request payload.""" From f878680838ac8191d511ebbf6ea5dd060d581d35 Mon Sep 17 00:00:00 2001 From: node9 Date: Wed, 8 Apr 2026 16:51:18 +0300 Subject: [PATCH 10/11] =?UTF-8?q?docs+test:=20move=20=E2=9A=A0=EF=B8=8F=20?= =?UTF-8?q?warnings=20to=20top=20of=20evaluate=20docstring;=20harden=20mal?= =?UTF-8?q?formed-response=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- node9/_client.py | 18 ++++++++++-------- tests/test_client.py | 20 +++++++++++++------- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/node9/_client.py b/node9/_client.py index 0c634fe..08ba8b2 100644 --- a/node9/_client.py +++ b/node9/_client.py @@ -267,6 +267,15 @@ def _evaluate_cloud(tool_name: str, args: dict[str, Any], run_id: str = "") -> N def evaluate(tool_name: str, args: dict[str, Any], *, run_id: str = "") -> None: """ + ⚠️ evaluate() is a low-level primitive. Unlike @protect it does NOT run DLP + scanning or inject agent context (run_id, agent_name) automatically. Use + @protect for full protection; use evaluate() only when decorator composition + is not possible (e.g. dynamic tool dispatch in custom frameworks). + + ⚠️ Default is fail-open: when neither daemon nor API key is available, + actions are auto-approved (offline audit mode). The exception is + policy == "require_approval" which raises DaemonNotFoundError (fail-closed). + Sends the action to node9 for audit / approval. Routing (first match wins): @@ -277,9 +286,7 @@ def evaluate(tool_name: str, args: dict[str, Any], *, run_id: str = "") -> None: Fail behaviour: - Daemon unreachable at call time → offline mode (fail-open, auto-approve) - ⚠️ Exception: policy == "require_approval" → DaemonNotFoundError (fail-closed). - Auto-approving when approval is explicitly required would be a silent - security bypass; operators must fix the connectivity issue instead. + Exception: policy == "require_approval" → DaemonNotFoundError (fail-closed). - Daemon dies mid-request → DaemonNotFoundError or ActionDeniedException (fail-closed: no auto-approve path exists once a request_id is acquired) - Daemon timeout / connection closed → ActionDeniedException (fail-closed) @@ -288,11 +295,6 @@ def evaluate(tool_name: str, args: dict[str, Any], *, run_id: str = "") -> None: Timeouts: see _CHECK_TIMEOUT (initial connection) and _WAIT_TIMEOUT (human decision wait) module constants for current values. - ⚠️ evaluate() is a low-level primitive. Unlike @protect it does NOT run DLP - scanning or inject agent context (run_id, agent_name) automatically. Use - @protect for full protection; use evaluate() only when decorator composition - is not possible (e.g. dynamic tool dispatch in custom frameworks). - Raises ActionDeniedException if the action is denied. """ if _SKIP: diff --git a/tests/test_client.py b/tests/test_client.py index d24c545..48545ac 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -16,9 +16,9 @@ def test_evaluate_importable_from_public_api(): separately to verify the export is intentional and discoverable by linters and import-* consumers. - Fail-open / offline-mode coverage lives in TestOfflineMode (4 tests), which - verifies auto-approve behavior, audit log writes, and the require_approval - RuntimeWarning path. + Fail-open / offline-mode coverage lives in TestOfflineMode, which verifies + auto-approve behavior, audit log writes, and require_approval fail-closed path + (raises DaemonNotFoundError — see test_offline_with_require_approval_policy_raises). """ import node9 from node9 import evaluate as pub_evaluate @@ -380,7 +380,10 @@ def capturing_urlopen(req, timeout): mock_check.assert_not_called() assert captured_headers, "urlopen was never called" - auth = captured_headers[0].get("Authorization", "") + # urllib capitalizes header keys (e.g. "authorization" → "Authorization"), + # so use case-insensitive lookup to avoid false negatives. + headers_lower = {k.lower(): v for k, v in captured_headers[0].items()} + auth = headers_lower.get("authorization", "") assert auth == "Bearer test-key-abc", f"Expected Bearer token, got: {auth!r}" def test_saas_denial_raises_action_denied_exception(self, monkeypatch): @@ -395,14 +398,17 @@ def test_saas_denial_raises_action_denied_exception(self, monkeypatch): assert "Blocked by policy" in str(exc.value) def test_saas_malformed_response_raises(self, monkeypatch): - """SaaS response missing 'approved' key must not silently auto-approve.""" + """SaaS response missing 'approved' key must raise ActionDeniedException, not auto-approve. + + _evaluate_cloud logic: approved missing → falsy; pending missing → falsy → + raises ActionDeniedException("Denied by Node9 policy"). No fall-through to offline mode. + """ monkeypatch.setenv("NODE9_API_KEY", "test-key") monkeypatch.setenv("NODE9_API_URL", "https://api.node9.ai/api/v1/intercept") - # Missing both 'approved' and 'pending' — should raise, not auto-approve malformed = _make_response({"status": "unknown"}) with patch("urllib.request.urlopen", return_value=malformed): - with pytest.raises((ActionDeniedException, RuntimeError)): + with pytest.raises(ActionDeniedException): evaluate("bash", {"command": "ls"}) def test_saas_run_id_defaults_to_empty_string(self, monkeypatch): From b5af1509963bffb5272fb2a94fb35a481360d720 Mon Sep 17 00:00:00 2001 From: node9 Date: Wed, 8 Apr 2026 16:53:49 +0300 Subject: [PATCH 11/11] fix: document and test that require_approval raise skips audit log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The raise DaemonNotFoundError in _offline_audit for require_approval policy intentionally skips the audit log — the action was not approved so logging it as 'allow' would be forensically wrong. Add explanatory comment in _offline_audit and a regression test confirming the audit log is not written when require_approval raises. Co-Authored-By: Claude Sonnet 4.6 --- node9/_client.py | 4 ++++ tests/test_client.py | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/node9/_client.py b/node9/_client.py index 08ba8b2..8e7d5a6 100644 --- a/node9/_client.py +++ b/node9/_client.py @@ -132,6 +132,10 @@ def _offline_audit(tool_name: str, args: dict[str, Any], run_id: str) -> None: # Fail-closed: require_approval means governance must be enforced. # Auto-approving silently would contradict the policy name and create # a false sense of security. Raise so operators see the misconfiguration. + # + # Intentionally no audit log entry here: the action was NOT approved — + # raising is the terminal path. The operator must fix the connectivity + # issue (start daemon or set NODE9_API_KEY) and retry. raise DaemonNotFoundError(DAEMON_PORT) audit_dir = os.path.join(os.path.expanduser("~"), ".node9") os.makedirs(audit_dir, exist_ok=True) diff --git a/tests/test_client.py b/tests/test_client.py index 48545ac..bd16748 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -315,6 +315,26 @@ def test_offline_mode_auto_approves(self, monkeypatch, tmp_path): result = evaluate("bash", {"command": "ls"}) assert result is None + def test_offline_require_approval_does_not_write_audit_log(self, monkeypatch, tmp_path): + """require_approval raise must NOT write an audit log entry. + + The action was not approved — logging it as 'allow' would be forensically + wrong and misleading. Operators should see the raised exception, fix + connectivity, and retry. + """ + import node9._config as cfg + monkeypatch.delenv("NODE9_API_KEY", raising=False) + monkeypatch.delenv("NODE9_AUTO_START", raising=False) + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.setattr(cfg, "AGENT_POLICY", "require_approval") + audit_path = tmp_path / ".node9" / "audit.log" + + with patch("node9._client._daemon_reachable", return_value=False): + with pytest.raises(DaemonNotFoundError): + evaluate("deploy", {"target": "prod"}) + + assert not audit_path.exists(), "Audit log must not be written when require_approval raises" + def test_offline_with_require_approval_policy_raises(self, monkeypatch, tmp_path): """require_approval + no daemon/API key must raise DaemonNotFoundError (fail-closed).