Skip to content

Add configurable TLS verification for all HTTP transports#43

Open
arseniy-pplx wants to merge 1 commit into
TangleML:masterfrom
arseniy-pplx:transfer/tls-ca-bundle
Open

Add configurable TLS verification for all HTTP transports#43
arseniy-pplx wants to merge 1 commit into
TangleML:masterfrom
arseniy-pplx:transfer/tls-ca-bundle

Conversation

@arseniy-pplx

@arseniy-pplx arseniy-pplx commented Jul 20, 2026

Copy link
Copy Markdown

Summary

Adds global, process-wide TLS verification controls to the CLI so a single trust policy applies uniformly across every HTTP transport: the static requests client, the dynamic httpx operation requests, and the OpenAPI schema fetch/refresh that runs during command construction.

Two global options are accepted before any subcommand:

  • --ca-bundle PATH — use a PEM CA bundle as the TLS trust store.
  • --verify-tls / --no-verify-tls — enable or disable certificate verification. Absence is an unset sentinel (not implicit True), so it does not override environment configuration.

Resolution is centralized in api_transport.resolve_verify(). Precedence: explicit Python verify= argument > global CLI flags > TANGLE_API_CA_BUNDLE > TANGLE_API_VERIFY_TLS > secure default. A defaulted (absent) flag never overrides the environment.

Context

The schema discovery that builds the dynamic api commands runs before normal Cyclopts dispatch, so the global flags are pre-parsed from argv in build_app() and installed via configure_cli_verify() before the api app is constructed. This makes the chosen trust policy reach even the pre-dispatch network call, without threading a verify argument through every call site.

Validation happens before any network request: an invalid, missing, or directory --ca-bundle path fails fast with one concise error; --ca-bundle combined with an explicit --no-verify-tls is rejected as contradictory, while --ca-bundle with --verify-tls is redundant but accepted.

--no-verify-tls (and TANGLE_API_VERIFY_TLS=0) is for local development only and intentionally preserves the urllib3 InsecureRequestWarning. The root help and README document the flags, environment variables, full precedence, accepted placement (before the subcommand), examples, and the security warning.

Testing

  • uv run pytest tests/test_tls_verification.py tests/test_packaging.py
  • new TLS tests cover configure_cli_verify precedence/conflict/invalid-CA/reset; argv pre-parse and stop-at-subcommand; override propagation to the static client, request_operation, and fetch_schema; real-HTTPS private-CA success, default-trust failure, and verify-off success for the schema/dynamic/requests paths; and CLI subprocess tests for api refresh under CA-bundle / default / no-verify, the ca-bundle+no-verify conflict, and root help listing the global flags
  • the private-CA test fixture now emits a standards-conformant leaf certificate (basicConstraints, keyUsage, extendedKeyUsage, subject/authority key identifiers) so real-HTTPS verification succeeds under the stricter OpenSSL validation in Python 3.13
  • CI runs the suite on both Python 3.12 and 3.13 so the TLS trust behavior is pinned across interpreters
  • uvx ruff check on the touched files
  • uv lock --check; uv build; CLI smoke check
  • git diff --check

@arseniy-pplx
arseniy-pplx marked this pull request as ready for review July 20, 2026 18:10
Comment thread tests/test_tls_verification.py Outdated
server_csr = directory / "server.csr"
server_crt = directory / "server.crt"
ext = directory / "ext.cnf"
ext.write_text("subjectAltName=DNS:localhost,IP:127.0.0.1\n", encoding="utf-8")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(AI-assisted)

[non-blocking] Could we make this generated certificate valid under Python 3.13’s TLS verification as well?

The full GitHub suite passes on its current Python 3.12 job, but the seven CA-bundle success cases fail locally on Python 3.13.12 with CERTIFICATE_VERIFY_FAILED: Missing Authority Key Identifier. Since the package declares requires-python = ">=3.10", 3.13 is currently within the advertised range.

Adding authorityKeyIdentifier=keyid,issuer to the leaf extension file (ideally with explicit basicConstraints=CA:FALSE, key usage, and server-auth EKU), then adding Python 3.13 to the CI matrix, would pin this portability contract. Alternatively, if 3.13 is not supported yet, the metadata/docs should narrow that expectation.


def main() -> None:
build_app()()
build_app().meta()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(AI-assisted)

Please coordinate this entrypoint with #46 rather than resolving the conflict by choosing one side.

This .meta() dispatch is required so the root TLS flags are installed before dynamic schema discovery. In sibling PR #46, main() instead enters SystemExit(run()), and run() dispatches build_app()(tokens). Git reports content conflicts between the two branches in cli.py, client.py, and api_transport.py.

Taking #46’s entrypoint as-is drops this pre-dispatch TLS path; taking #43’s entrypoint bypasses #46’s clean transport-error boundary. Could the later PR rebase and explicitly compose them so the guarded runner dispatches the meta app? A combined test should cover both --ca-bundle ... api refresh and a static-client connection failure after both changes are present.

Sibling: #46

Introduce a single TLS verification contract shared by the requests
client, the httpx schema/operation transport, and the dynamic-discovery
client. TANGLE_API_CA_BUNDLE verifies against a custom CA bundle and
TANGLE_API_VERIFY_TLS toggles verification, with verification enabled by
default. Precedence is explicit argument, CA bundle, verify flag, then
the secure default; unset settings preserve requests' environment and
caller-supplied session behavior.
@arseniy-pplx
arseniy-pplx force-pushed the transfer/tls-ca-bundle branch from f4da2d0 to d5aede2 Compare July 24, 2026 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants