fix: merge latest dev updates into main

[node9ai]

Auto-generated PR

Merge latest dev changes into main to trigger a release.

⚠️ Important: When you click Squash and Merge, ensure the commit message starts with:

• fix: to publish a Patch release (0.0.X)
• feat: to publish a Minor release (0.X.0)
  If it starts with chore:, no PyPI package will be published!

[node9ai]

🤖 Claude Code Review

Code Review: Node9 Python SDK — Cloud Routing PR

Security Issues (High Priority)

1. API key leak via empty string check

api_key = os.environ.get("NODE9_API_KEY", "")

If NODE9_API_KEY is set but empty (export NODE9_API_KEY=), evaluate() will route to cloud, send an empty Bearer token, and depending on server behavior may silently fail or get a surprising response. The if os.environ.get("NODE9_API_KEY"): guard in evaluate() catches the empty-string case correctly, but _evaluate_cloud is still callable with an empty key if called directly. Consider validating api_key is non-empty at the top of _evaluate_cloud and raising early.

2. No TLS certificate validation hardening
urllib.request.urlopen uses the default SSL context. For a security library, you should be explicit — consider asserting https:// scheme before sending credentials, or raising if api_url is not HTTPS. A misconfigured NODE9_API_URL=http://... would send the Bearer token in plaintext.

3. ci-context.json is blindly trusted
_read_ci_context() reads arbitrary JSON from ~/.node9/ci-context.json and injects it into the cloud payload without any validation or size cap. If this file is attacker-controlled (e.g., in a shared CI environment), it could be used to poison the approval context seen by the human reviewer. At minimum, cap the size and validate expected keys.

4. Default URL points to dev-api

api_url = os.environ.get("NODE9_API_URL", "https://dev-api.node9.ai/api/v1/intercept")

Shipping a dev endpoint as the production default is a significant mistake. If NODE9_API_URL is not set in prod, requests silently go to a dev/staging environment where approval semantics may differ. This default should be the production URL, or it should raise if neither env var is set.

---

Correctness & Edge Cases

5. Poll URL construction is fragile

status_url = f"{api_url}/status/{request_id}"

If api_url has a trailing slash (e.g., user sets NODE9_API_URL=https://api.node9.ai/api/v1/intercept/), this produces a double-slash path. Normalize the base URL before constructing the status URL.

6. request_id not sanitized in URL
If requestId from the server contains path traversal characters (e.g., ../../admin), the constructed status_url could point to an unintended endpoint. Validate that request_id matches an expected format (UUID) before using it in the URL.

7. os.uname() not available on Windows
os.uname() raises AttributeError on Windows. Since this is a Python SDK intended for LangChain/CrewAI agents (which run on Windows), use platform.node() and platform.system() instead.

8. Poll loop swallows all network errors silently
The continue on URLError/HTTPException means transient 401/403 errors during polling are ignored for the full 10-minute window. A 401 (invalid key) will spin for 10 minutes before timing out. At minimum, break out on authentication errors.

---

API Design / Usability

9. Polling duration is hardcoded
The 10-minute timeout is not configurable. Agents running in short-lived CI jobs (e.g., 5-minute timeout) will hang until the job is killed. Expose a NODE9_CLOUD_TIMEOUT env var.

10. print() for status output
Using print() directly is inappropriate for a library — it pollutes stdout for any agent framework that captures output. Use logging.getLogger("node9").info(...) instead.

---

Test Coverage Gaps

• No test for NODE9_API_KEY set but empty string
• No test for NODE9_API_URL with trailing slash
• No test for os.uname() failure (Windows path)
• No test for poll loop receiving HTTP 401 — verify it doesn't spin for 10 minutes
• No test for malformed requestId in status URL construction
• No test for ci-context.json with oversized or unexpected content

---

Minor

The paths-ignore addition to the workflow is correct and intentional — no issues there. Version bump to 0.1.1 is appropriate.

Summary: The cloud routing logic has several real security and correctness issues that should be addressed before merging, particularly the dev-default URL, Windows incompatibility, unsanitized requestId in URL construction, and missing HTTPS enforcement.

---

Automated review by Claude Sonnet

[node9ai]

🤖 Claude Code Review

Code Review: Node9 Python SDK — Cloud Routing PR

Security Issues (Critical)

1. cwd leakage in payload
os.getcwd() is sent to the SaaS endpoint unconditionally. In CI environments this likely contains repository paths, but on developer machines it could expose sensitive directory structures to the remote server. This should be opt-in or stripped.

2. NODE9_CLOUD_TIMEOUT is unsanitized

poll_timeout = int(os.environ.get("NODE9_CLOUD_TIMEOUT", "600"))

No bounds checking. A value of 0 or negative makes the while loop never execute (silent immediate timeout → ActionDeniedException). A value of 99999999 creates an indefinite block. Clamp to a reasonable range (e.g., 30–3600).

3. api_url path traversal via request_id

status_url = f"{api_url}/status/{request_id}"

Even with the regex _REQUEST_ID_RE, this constructs a URL by string interpolation. The regex allows - which is fine, but api_url itself comes from an env var. If someone sets NODE9_API_URL=https://api.node9.ai/api/v1/intercept/../../../../admin, the rstrip("/") doesn't help. Use urllib.parse.urljoin or validate api_url more strictly (hostname pinning or allowlist).

4. HTTPS check is bypassable

if not api_url.startswith("https://"):

"https://evil.com@api.node9.ai" passes this check. Consider parsing with urllib.parse.urlparse and validating scheme + netloc separately.

5. ci-context.json TOCTOU
There's a os.path.getsize() call followed by open(). On a system with an adversarial process, the file could be swapped between the size check and the read. Read the file first, then check length of bytes read. Minor in practice but relevant for a security library.

---

Correctness & Edge Cases

6. _evaluate_cloud uses _CHECK_TIMEOUT (5s) for initial POST
The initial intercept call uses timeout=_CHECK_TIMEOUT (5 seconds). For a SaaS that may queue the request and do processing, 5 seconds is very tight and will cause spurious RuntimeError on slow connections, with no retry logic.

7. Unknown polling statuses are silently ignored
If the server returns an unrecognized status (e.g., "PROCESSING", "QUEUED", or a future status), the loop just continues until timeout. This is probably intentional but should be documented. More importantly, a typo like "Approved" (wrong case) would silently timeout — consider case-normalizing status.

8. result.get("approved") truthiness check
If the server returns {"approved": false, "pending": false} without a reason, the code falls through to the pending check, then raises ActionDeniedException with the default reason. That's correct but fragile — the logic would be clearer as an explicit status field check rather than boolean flag juggling.

9. json.dumps(..., default=str) on args
This silently converts non-serializable objects to their string repr. For a security library intercepting tool calls, silently mangling the args before sending for approval is a correctness problem — the human approves a string representation, but the actual object executes. Should raise explicitly if args aren't JSON-serializable.

---

API Design

10. NODE9_SKIP=1 takes precedence over NODE9_API_KEY
The bypass check runs before the API key check. This is documented but easy to misconfigure in CI (accidentally set both). At minimum, log a warning when NODE9_SKIP=1 is active in an environment that also has NODE9_API_KEY.

11. print() for user feedback

print(f"🛡️  Node9: waiting for approval of '{tool_name}'...", flush=True)

This will pollute stdout in production agents (LangChain, CrewAI). Should use logging so integrators can suppress it.

---

Test Coverage Gaps

• No test for NODE9_CLOUD_TIMEOUT=0 or negative values
• No test for malformed NODE9_API_URL (non-HTTPS, authority confusion)
• No test for the TOCTOU path in _read_ci_context
• No test for unknown poll statuses (ensures they don't silently approve)
• No test covering NODE9_SKIP=1 + NODE9_API_KEY coexistence
• No test for non-serializable args with default=str

---

Minor

The workflow paths-ignore change is correct and intentional — no issues there.

Summary: The cloud routing feature has a solid structure, but several issues are significant for a security library: URL validation is insufficient, timeout is unbounded, and silent arg mangling during serialization is a correctness risk for the approval flow. These should be addressed before merge.

---

Automated review by Claude Sonnet

[node9ai]

🤖 Claude Code Review

Code Review: Node9 Python SDK — Cloud Routing PR

Overall Assessment

The core logic is reasonable, but there are several security and correctness issues that need attention given this is a security library.

---

Security Issues

1. cwd in payload is an information leak (Medium)
os.getcwd() is sent to the SaaS API with every request. In CI, this exposes internal directory structure. More critically, if an attacker can influence the working directory name, they can smuggle path information into approval context. At minimum, this should be opt-in or documented.

2. api_url HTTPS check is bypassable (Medium)

if not api_url.startswith("https://"):

This check happens after rstrip("/"), which is fine, but the validation is weak. A URL like https://evil.com?redir=https://api.node9.ai passes. You should parse with urllib.parse.urlparse and validate scheme == "https" and that the host is either the default or explicitly trusted. At minimum, consider locking to a fixed host unless overridden in a non-CI context.

3. NODE9_SKIP=1 bypass is still prioritized over cloud routing
The skip check happens before the NODE9_API_KEY check. In a CI environment where both could be set, the security check is silently bypassed. This ordering should at minimum be documented, but arguably NODE9_SKIP should be disabled when NODE9_API_KEY is present in a production/CI context.

4. _read_ci_context values are not sanitized
The allowlist covers keys but not values. A compromised or malicious ci-context.json can include arbitrarily large string values per key, embedded control characters, or JSON-injection-friendly content. Consider capping value length per key.

5. platform.node() leaks the hostname to the SaaS API
In many security contexts, this is sensitive. Should be opt-in or at least documented.

---

Correctness & Edge Cases

6. Poll timeout integer conversion can raise unhandled ValueError

int(os.environ.get("NODE9_CLOUD_TIMEOUT", "600"))

If NODE9_CLOUD_TIMEOUT is set to a non-integer string (e.g., "fast"), this raises ValueError with no user-friendly message. Wrap in try/except with a clear error.

7. _evaluate_cloud uses _CHECK_TIMEOUT (5s) for initial POST
The initial intercept POST uses the connection-check timeout. For a synchronous SaaS POST that may involve queueing logic, 5 seconds is likely too short. This should use a distinct, larger timeout constant.

8. status_url construction is fragile

status_url = f"{api_url}/status/{request_id}"

Since api_url is rstrip("/") processed, this should be fine — but if NODE9_API_URL is set to a base URL (e.g., https://api.node9.ai/api/v1), the status URL becomes https://api.node9.ai/api/v1/intercept/status/{id}, which may not be the intended endpoint. The status URL should be returned by the server in the initial response (result.get("statusUrl")), not constructed client-side.

9. result.get("approved") vs result.get("pending") logic
If the server returns neither approved: true nor pending: true, and no explicit deny signal, execution falls through to the polling loop with no requestId, which raises RuntimeError. This is safe but the error message could be clearer — it shows the raw result dict which may expose sensitive server internals in logs.

---

API Design

10. print() for status output is inappropriate for a library

print(f"🛡️  Node9: waiting for approval of '{tool_name}'...", flush=True)

Libraries should use logging, not print. This breaks log capture in LangChain/CrewAI integrations and can't be silenced without patching sys.stdout.

11. No way to inject a custom HTTP session/adapter
Using urllib.request directly makes it impossible for integrators to add proxies, retry logic, or custom TLS configuration without monkey-patching. This is a known pain point when integrating into enterprise environments.

---

Test Coverage Gaps

• No tests for _read_ci_context with oversized files, invalid JSON, non-dict JSON, or keys with oversized values
• No tests for NODE9_CLOUD_TIMEOUT with invalid values
• No tests for the polling loop timeout expiry path
• No test asserting that NODE9_SKIP=1 + NODE9_API_KEY set still bypasses (documents the behavior, which is currently undocumented and arguably wrong)
• No test for NODE9_API_URL with a non-HTTPS scheme

---

Minor

• The paths-ignore addition in the workflow is correct and uncontroversial.
• Version bump to 0.1.1 is appropriate.
• _REQUEST_ID_RE validation is a good security practice — no issues there.

---

Automated review by Claude Sonnet