fix(NBE-1377): keep TLS when DIODE_SKIP_TLS_VERIFY is set

[jajeffries]

Summary

• Align the Python Diode SDK with the Go SDK for DIODE_SKIP_TLS_VERIFY: the connection stays on grpcs/https and only certificate validation is skipped.
• Fixes deployments behind Traefik or other proxies that present self-signed certificates, where the previous behavior could downgrade to plaintext (h2c) and surface as 404 or protocol errors.
• Preserves encryption on the wire while allowing lab and staging setups that cannot use publicly trusted certs.

What changed

• parse_target now returns separate is_plaintext and tls_verify flags instead of treating skip-verify as plaintext.
• _open_grpc_channel uses TLS credentials with an optional peer certificate pin when verification is disabled; auth token URL scheme follows is_plaintext.
• Added skip_tls_verify constructor parameter and documented DIODE_SKIP_TLS_VERIFY in the README.

How tested

• pytest tests/test_client.py -q (133 passed)
• ruff check netboxlabs/diode/sdk/client.py tests/test_client.py

Linear

• Upstream: NBE-1377
• Customer context: ENGHLP-1703 (self-signed / Traefik TLS in front of Diode)

Made with Cursor

[github-actions]

Coverage Report

File	Stmts	Miss	Cover	Missing
/opt/hostedtoolcache/Python/3.11.16/x64/lib/python3.11/site-packages/netboxlabs/diode/sdk
chunking.py	33	1	97%	112
client.py	637	79	88%	125–126, 140, 144–170, 176–193, 226, 317–318, 337–339, 342–345, 619, 687, 692, 696, 785–786, 814–816, 846, 855, 859, 881, 902, 904, 978, 1009, 1017, 1079–1080, 1089, 1122, 1149, 1154–1155, 1184, 1189–1192
exceptions.py	44	3	93%	69, 82–83
TOTAL	729	83	89%

Tests	Skipped	Failures	Errors	Time
220	0 💤	0 ❌	0 🔥	1.911s ⏱️

[nbl-ai-review]

AI Code Review — risk tier: Full · dimensions run: correctness, security

Advisory only. A human owns the merge decision.

Summary — This is a net-positive change: it correctly stops downgrading to plaintext when DIODE_SKIP_TLS_VERIFY is set, matching the Go SDK and fixing the stated Traefik/self-signed-cert deployment issue. The new peer-certificate-pinning mechanism that makes this possible, however, introduces a real correctness risk (pinning a cert from a different connection than the one the gRPC channel actually uses) and a couple of narrower robustness gaps in the new socket/TLS probe code.

Findings

• [High] correctness — netboxlabs/diode/sdk/client.py:196 (_skip_verify_channel_credentials) — _fetch_peer_leaf_certificate opens its own short-lived probe connection to fetch one leaf certificate and pins it as root_certificates for the gRPC channel opened moments later on a separate connection. Against the exact scenario this PR targets — a load-balanced proxy (e.g. Traefik) with multiple backend instances each presenting a distinct self-signed cert — the probe may land on instance A while the real gRPC channel (which gRPC can independently re-resolve/reconnect over its lifetime) lands on instance B, causing TLS verification failures even with skip_tls_verify=True, including intermittent failures after a successful startup. Suggestion: either use CERT_NONE end-to-end for the real channel instead of pinning a single fetched cert, or explicitly document/enforce single-backend-only use and add a test covering the multi-instance case.

• [Medium] security/correctness — netboxlabs/diode/sdk/client.py:121 and :143 — Exception handling around the new cert-probe path is inconsistent with the rest of the SDK. _tls_server_name_from_cert_pem (line 121) parses attacker-controllable certificate bytes (fetched with verify_mode = ssl.CERT_NONE, so this is expected to be untrusted) using the private, unstable ssl._ssl._test_decode_cert API, but only catches ssl.SSLError — a malformed cert can raise a different exception type and crash the client, i.e. an on-path MITM (the exact condition skip-verify tolerates) can crash the process. Separately, the whole probe (_open_grpc_channel → _skip_verify_channel_credentials → _fetch_peer_leaf_certificate → _connect_socket, from line 143) performs blocking, synchronous, unretried network I/O directly inside __init__, and its raw socket/SSL/DNS failures propagate unwrapped instead of the DiodeConfigError this SDK otherwise uses for construction-time failures, and instead of the connect-on-first-use behavior every other channel path preserves. Suggestion: widen the except clause (or switch to the public cryptography library) around the private-API call and fail closed by returning None; wrap the socket/TLS probe's exceptions in DiodeConfigError for consistency with the rest of the SDK.

• [Low] security — netboxlabs/diode/sdk/client.py:158 (_connect_socket) — The loop reading the proxy CONNECT response accumulates into response via sock.recv(4096) with no cap until the \r\n\r\n terminator is seen. A misbehaving or malicious proxy that never sends the terminator while continuing to send data causes unbounded memory growth. Suggestion: cap the accumulated buffer (e.g. a few KB) and raise DiodeConfigError if the terminator isn't found within that bound.

_{🤖 AI Code Review · run · prompts: code-review/v1 · models: claude-sonnet-5/medium · context: description · 1 comment(s) · 0 prior AI review(s) · usage: $1.21 · 1457k in (90% cached) · 20k out}

[jajeffries]

Superseded by reticket to ENGHLP-1703; new PR will follow on branch fix/enghlp-1703-skip-tls-verify-parity.