Skip to content

Add --insecure flag to caib CLI for testing with self-signed certificates#84

Merged
bennyz merged 2 commits into
centos-automotive-suite:mainfrom
bkhizgiy:insecure
Feb 8, 2026
Merged

Add --insecure flag to caib CLI for testing with self-signed certificates#84
bennyz merged 2 commits into
centos-automotive-suite:mainfrom
bkhizgiy:insecure

Conversation

@bkhizgiy

@bkhizgiy bkhizgiy commented Feb 8, 2026

Copy link
Copy Markdown
Contributor

Adds global --insecure flag to skip TLS certificate verification when connecting to Build API servers with self-signed or untrusted certificates.

Usage:
Via command-line flag
caib list --server https://build-api.local --insecure
Via environment variable

export CAIB_INSECURE=true
caib list --server https://build-api.local

Combined with login
caib login https://build-api.local --insecure

Summary by CodeRabbit

Release Notes

  • New Features
    • Added --insecure command-line flag to disable TLS certificate verification
    • Support for CAIB_INSECURE environment variable as an alternative to the flag
    • TLS verification skip applies consistently to authentication, API operations, and logging

Signed-off-by: Bella Khizgiyaev <bkhizgiy@redhat.com>
Signed-off-by: Bella Khizgiyaev <bkhizgiy@redhat.com>
@bkhizgiy bkhizgiy requested a review from bennyz February 8, 2026 17:12
@coderabbitai

coderabbitai Bot commented Feb 8, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This pull request introduces insecure TLS verification support across the CAIB CLI tooling. It adds a new --insecure flag (with CAIB_INSECURE environment variable support) that allows skipping TLS certificate verification. The flag is threaded through authentication flows, HTTP client initialization, and catalog command handlers to enable optional TLS bypass for testing or development scenarios.

Changes

Cohort / File(s) Summary
Authentication Layer
cmd/caib/auth/config.go, cmd/caib/auth/config_test.go
Extended GetOIDCConfigFromAPI signature to accept insecureSkipTLS bool parameter; refactored TLS config construction to conditionally set InsecureSkipVerify. Test calls updated to pass false argument.
OIDC Authentication
cmd/caib/auth/oidc.go
Updated NewOIDCAuth constructor to accept insecureSkipTLS bool parameter and store it in struct; applied conditional TLS verification skipping in HTTP transport creation for token exchange and discovery.
Auth Wrapper Layer
cmd/caib/auth/wrapper.go, cmd/caib/auth/wrapper_test.go
Added insecureSkipTLS parameter to GetTokenWithReauth and CreateClientWithReauth; threaded parameter through to OIDC flows and client configuration. Test calls updated accordingly.
Catalog Utilities
cmd/caib/catalog/catalog.go
Introduced three new helper functions: getInsecureSkipTLS(cmd) to read flag/env var, envBool(key) for environment variable parsing, and newHTTPClient(insecureSkipTLS) to construct clients with optional TLS bypass. Updated imports to include crypto/tls, net/http, os, strconv, strings.
Catalog Commands
cmd/caib/catalog/add.go, cmd/caib/catalog/get.go, cmd/caib/catalog/list.go, cmd/caib/catalog/publish.go, cmd/caib/catalog/remove.go, cmd/caib/catalog/verify.go
Unified pattern across six command handlers: renamed unnamed cobra.Command parameter to cmd to enable flag access; replaced default http.Client{} initialization with newHTTPClient(getInsecureSkipTLS(cmd)).
Main CLI Entry Point
cmd/caib/main.go
Added persistent boolean flag --insecure with CAIB_INSECURE environment variable fallback via new envBool helper. Propagated insecureSkipTLS through authentication flows (GetTokenWithReauth), API client setup (WithInsecureTLS), and log streaming clients across multiple call sites.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Suggested reviewers

  • bennyz

Poem

🐰 Through TLS gates we cautiously hop,
With flags to skip verification's stop,
Each command now reads the insecure way,
Testing paths made safe and gay!
Certificates trusted—or skipped for the day! 🔓

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.87% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main feature addition: adding an --insecure flag to the caib CLI for testing with self-signed certificates, which aligns with the primary objective of the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
cmd/caib/auth/config.go (1)

22-25: Consider setting MinVersion on the TLS config.

When insecureSkipTLS is false, the tls.Config relies on Go's default minimum TLS version. Explicitly setting MinVersion hardens the configuration and silences static analysis warnings (CWE-327).

🔒 Proposed fix
-	tlsConfig := &tls.Config{RootCAs: pool}
+	tlsConfig := &tls.Config{RootCAs: pool, MinVersion: tls.VersionTLS12}
 	if insecureSkipTLS {
 		tlsConfig.InsecureSkipVerify = true
 	}
cmd/caib/auth/config_test.go (1)

46-46: No test coverage for insecureSkipTLS = true.

All test cases pass false. Consider adding at least one test using httptest.NewTLSServer with insecureSkipTLS = true to verify the skip-verify path works end-to-end and doesn't regress.

cmd/caib/catalog/catalog.go (2)

95-105: newHTTPClient creates a bare Transport, losing DefaultTransport settings.

When insecureSkipTLS is true, a raw &http.Transport{} is used instead of cloning http.DefaultTransport. This loses proxy settings, HTTP/2 support, connection pooling, and timeouts. Contrast with WithInsecureTLS() in internal/buildapi/client/client.go which correctly clones DefaultTransport.

♻️ Proposed fix
 func newHTTPClient(insecureSkipTLS bool) *http.Client {
 	if insecureSkipTLS {
+		transport := http.DefaultTransport.(*http.Transport).Clone()
+		transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
 		return &http.Client{
-			Transport: &http.Transport{
-				TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
-			},
+			Transport: transport,
 		}
 	}
 	return &http.Client{}
 }

82-93: envBool is duplicated in cmd/caib/main.go (lines 104–115).

Both implementations are identical. Consider extracting this into a shared package (e.g., alongside config) to avoid drift.

cmd/caib/auth/wrapper_test.go (1)

13-13: Test signature updates look correct.

All call sites pass false for the new insecureSkipTLS parameter. Consider adding at least one test case with insecureSkipTLS: true to verify the WithInsecureTLS() option is applied to the client.

Also applies to: 21-21, 29-29, 38-38

cmd/caib/auth/oidc.go (1)

280-288: Bare &http.Transport{} loses DefaultTransport settings (same pattern as in catalog.go).

Both exchangeCodeForToken (Line 280) and getDiscovery (Line 330) create a bare transport, losing proxy, HTTP/2, and connection pooling settings from DefaultTransport. Consider cloning http.DefaultTransport instead.

♻️ Proposed fix for exchangeCodeForToken
-	transport := &http.Transport{}
+	transport := http.DefaultTransport.(*http.Transport).Clone()
 	if a.insecureSkipTLS {
 		transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
 	}
♻️ Proposed fix for getDiscovery (Line 330)
-	transport := &http.Transport{}
+	transport := http.DefaultTransport.(*http.Transport).Clone()
 	if a.insecureSkipTLS {
 		transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
 	}
cmd/caib/main.go (1)

1358-1368: Bare &http.Transport{} for log streaming also loses DefaultTransport settings.

Same concern as flagged in catalog.go and oidc.go. The custom timeout fields are good, but proxy and HTTP/2 support from DefaultTransport are lost. Consider cloning DefaultTransport and then setting the additional fields.

♻️ Proposed fix
-	logTransport := &http.Transport{
-		ResponseHeaderTimeout: 30 * time.Second,
-		IdleConnTimeout:       2 * time.Minute,
-	}
+	logTransport := http.DefaultTransport.(*http.Transport).Clone()
+	logTransport.ResponseHeaderTimeout = 30 * time.Second
+	logTransport.IdleConnTimeout = 2 * time.Minute
 	if insecureSkipTLS {
 		logTransport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
 	}

The same applies to flashLogTransport at lines 2036–2042.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@bennyz bennyz merged commit b6de1d5 into centos-automotive-suite:main Feb 8, 2026
4 checks passed
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