Skip to content

feat(cli): add --insecure and --ca-bundle for self-signed CAs#232

Merged
ian-flores merged 5 commits intomainfrom
feat-ssl-cert-flags
Apr 30, 2026
Merged

feat(cli): add --insecure and --ca-bundle for self-signed CAs#232
ian-flores merged 5 commits intomainfrom
feat-ssl-cert-flags

Conversation

@ian-flores
Copy link
Copy Markdown
Collaborator

Summary

  • Adds --insecure and --ca-bundle PATH flags that flow end-to-end: CLI → VIPConfig [tls] section → all httpx clients (BaseClient.verify) → Playwright (ignore_https_errors, NODE_EXTRA_CA_CERTS with save/restore) → Keycloak credential provisioning.
  • All ad-hoc httpx call sites in auth.py, credentials.py, connect.py, helpers.py, and test_https.py honor the new settings.
  • Path quoting uses json.dumps for TOML safety; a _resolve_ca_bundle validator surfaces clean CLI errors when the file is missing; using both flags together emits a UserWarning.
  • Eighteen new selftests cover the round-trip, env handling, fixture restore, and CLI temp-config generation.

Builds additively on the TLS diagnostics improvements from #198 — the diagnostic step in test_ssl.py is untouched.

Closes #224.

Test plan

  • uv run ruff check src/ src/vip_tests/ selftests/ examples/
  • uv run ruff format --check src/ src/vip_tests/ selftests/ examples/
  • uv run pytest selftests/ -v (338 passed)
  • Sanity: vip verify --connect-url https://example.invalid --insecure --no-auth -- --collect-only succeeds

- Use TOML literal-string quoting for ca_bundle path in _generate_temp_config
  to avoid backslash/quote escaping issues on Windows paths
- Save and restore NODE_EXTRA_CA_CERTS in a finally block in both
  start_interactive_auth and start_headless_auth to prevent env leakage
- Wire ca_bundle into browser_context_args fixture via NODE_EXTRA_CA_CERTS
  so Playwright tests trust the custom CA even when auth is cached
- Expose a public verify property on BaseClient; update workbench/conftest.py
  to use it instead of _verify
- Emit a UserWarning when both --insecure and --ca-bundle are passed
- Add two selftests: ca_bundle sets NODE_EXTRA_CA_CERTS and is restored after
- Validate ca_bundle path existence in load_config for early, clear errors
- Use json.dumps for ca_bundle path in TOML temp config to safely handle
  paths containing single quotes, backslashes, or other special characters
- Restore NODE_EXTRA_CA_CERTS via request.addfinalizer in browser_context_args
  fixture to prevent env leakage when ca_bundle is set
- Catch ValueError from load_config (e.g. missing ca_bundle file) at the
  CLI boundary in _print_skip_notes and _check_credentials for clean error UX
- Add selftest for both-flags-set UserWarning in TestVerifyLocalTLSFlags
- Add selftest for _resolve_ca_bundle failure path in TestLoadConfigTLS
- Document that verify is HTTP-only in check_data_source_connectivity
- Tighten test_no_insecure_does_not_set_ignore_https_errors to assert False
  explicitly rather than relying on falsy check
Copilot AI review requested due to automatic review settings April 30, 2026 00:35
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds end-to-end TLS configuration controls to support self-signed / corporate CA environments by flowing new settings from CLI/config into httpx clients and Playwright, plus updating tests and examples accordingly.

Changes:

  • Introduces [tls] config (insecure, ca_bundle) in VIPConfig + vip.toml.example, with validation for missing CA bundle files.
  • Wires TLS settings into BaseClient/product clients, ad-hoc httpx call sites, and Playwright contexts (ignore_https_errors, NODE_EXTRA_CA_CERTS with restore).
  • Adds/updates selftests to cover config parsing, temp-config generation, verify propagation, and Playwright env handling.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
vip.toml.example Documents new [tls] configuration options.
src/vip/config.py Adds TLS fields to VIPConfig, parses [tls], validates CA bundle path.
src/vip/cli.py Adds --insecure / --ca-bundle, writes [tls] into generated temp config.
src/vip/clients/base.py Centralizes httpx verify derivation and exposes it via BaseClient.verify.
src/vip/clients/connect.py Ensures ad-hoc httpx.get() calls honor the client TLS verify setting.
src/vip/clients/workbench.py Plumbs TLS options into the Workbench client constructor.
src/vip/clients/packagemanager.py Plumbs TLS options into the Package Manager client constructor.
src/vip/auth.py Adds TLS handling for Playwright contexts + NODE_EXTRA_CA_CERTS save/restore; TLS-aware API key deletion.
src/vip/plugin.py Passes TLS settings into interactive/headless auth setup.
src/vip/verify/credentials.py Adds TLS-aware httpx clients for Keycloak provisioning flows.
src/vip_tests/conftest.py Passes TLS options into session clients and Playwright browser contexts; manages NODE_EXTRA_CA_CERTS.
src/vip_tests/security/test_https.py Makes HTTP requests honor TLS settings; retains cert-trust classification behavior.
src/vip_tests/helpers.py Adds verify plumbing for HTTP data source connectivity checks.
src/vip_tests/connect/test_data_sources.py Computes verify behavior from vip_config and passes into helper.
src/vip_tests/workbench/conftest.py Uses a temp httpx client inheriting TLS verify from the session client.
selftests/test_config.py Adds coverage for TLS defaults and TOML parsing + missing CA bundle error.
selftests/test_clients_connect.py Adds coverage ensuring verify is passed through on fetch_content().
selftests/test_cli_verify.py Adds coverage for TLS flags being encoded into the generated temp config + warning.
selftests/test_auth.py Adds coverage for ignore_https_errors and NODE_EXTRA_CA_CERTS set/restore behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/vip/cli.py Outdated
Comment on lines +375 to +382
if insecure or ca_bundle:
lines.append("[tls]")
if insecure:
lines.append("insecure = true")
if ca_bundle:
import json as _json

lines.append(f"ca_bundle = {_json.dumps(str(ca_bundle))}")
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

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

In the --insecure + --ca-bundle case, the earlier warning says the CA bundle is ignored for verification, but this block still writes ca_bundle into the temp TOML. Because load_config() validates ca_bundle existence, a missing file can still make vip verify --insecure --ca-bundle <missing> fail. Consider not emitting ca_bundle when insecure is set (or otherwise preventing CA-bundle validation from blocking insecure mode).

Suggested change
if insecure or ca_bundle:
lines.append("[tls]")
if insecure:
lines.append("insecure = true")
if ca_bundle:
import json as _json
lines.append(f"ca_bundle = {_json.dumps(str(ca_bundle))}")
effective_ca_bundle = None if insecure else ca_bundle
if insecure or effective_ca_bundle:
lines.append("[tls]")
if insecure:
lines.append("insecure = true")
if effective_ca_bundle:
import json as _json
lines.append(f"ca_bundle = {_json.dumps(str(effective_ca_bundle))}")

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in 5839a0a. When insecure=True, effective_ca_bundle is set to None so the ca_bundle line is never written to the temp config. A new selftest (test_insecure_omits_ca_bundle_from_temp_config) asserts this with a deliberately missing PEM path to prove load_config() validation is not triggered.

Comment thread src/vip_tests/security/test_https.py Outdated
Comment on lines +55 to +59
if vip_config.insecure:
verify = False
elif vip_config.ca_bundle is not None:
verify = str(vip_config.ca_bundle)
else:
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

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

The TLS verify selection logic is duplicated here (and again in inspect_headers). Consider extracting a small helper (e.g., a function that returns the correct httpx verify value from vip_config) to avoid copy/paste drift as TLS options evolve.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in 5839a0a. Extracted _verify_from_config(vip_config) -> bool | str as a module-level helper and replaced both duplicated blocks in make_http_request (line ~56) and inspect_headers (line ~101) with a single _verify_from_config(vip_config) call.

- Gate ca_bundle write on not insecure in _generate_temp_config so that
  vip verify --insecure --ca-bundle <missing> no longer fails config
  validation when the user's intent is to skip TLS entirely.
- Extract _verify_from_config helper in test_https.py and use it in
  both make_http_request and inspect_headers to eliminate the duplicated
  verify-selection logic flagged by Copilot.
- Add selftest asserting ca_bundle is absent from temp config when
  insecure=True, even when --ca-bundle is also passed.
@ian-flores ian-flores marked this pull request as ready for review April 30, 2026 02:05
@ian-flores ian-flores requested a review from samcofer April 30, 2026 02:09
@ian-flores ian-flores merged commit 9959204 into main Apr 30, 2026
20 checks passed
@ian-flores ian-flores deleted the feat-ssl-cert-flags branch April 30, 2026 02:10
@github-actions
Copy link
Copy Markdown

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-04-30 02:10 UTC

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.

Add --insecure flag or SSL_CERT_FILE support for self-signed/corporate CA certificates

2 participants