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') 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", diff --git a/node9/_client.py b/node9/_client.py index 92f7dc6..8e7d5a6 100644 --- a/node9/_client.py +++ b/node9/_client.py @@ -129,14 +129,14 @@ 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. + # + # 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) audit_path = os.path.join(audit_dir, "audit.log") @@ -271,12 +271,34 @@ 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 + ⚠️ 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): + 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 unreachable at call time → offline mode (fail-open, auto-approve) + 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) + - SaaS HTTP error or timeout → RuntimeError (propagates to caller) + + Timeouts: see _CHECK_TIMEOUT (initial connection) and _WAIT_TIMEOUT + (human decision wait) module constants for current values. + Raises ActionDeniedException if the action is denied. """ if _SKIP: diff --git a/tests/test_client.py b/tests/test_client.py index 0b48e7b..bd16748 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -8,6 +8,23 @@ from node9._client import evaluate +def test_evaluate_importable_from_public_api(): + """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, 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 + assert callable(pub_evaluate) + assert "evaluate" in node9.__all__ + def _make_response(data: dict): """Create a mock urllib response.""" @@ -298,22 +315,159 @@ 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_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). + + 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: + """Tests for the NODE9_API_KEY → SaaS routing 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") + + http_error = urllib.error.HTTPError( + url="https://api.node9.ai/api/v1/intercept", + code=401, + msg="Unauthorized", + hdrs=http.client.HTTPMessage(), + 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_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}) + + 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"}) + mock_check.assert_not_called() + + assert captured_headers, "urlopen was never called" + # 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): + """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 str(exc.value) + + def test_saas_malformed_response_raises(self, monkeypatch): + """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") + + malformed = _make_response({"status": "unknown"}) + with patch("urllib.request.urlopen", return_value=malformed): + with pytest.raises(ActionDeniedException): + 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.""" + 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: