feat(tpagectl): add TLS verification controls #597

Merged

fuzzy merged 1 commit from feat/cli-tls-controls into main

2026-08-08 05:09:55 +00:00

automation commented

2026-08-08 05:06:23 +00:00

Owner

What

Implements Phase 6 TLS verification controls (roadmap #261).

--insecure (#258) — new root flag (and insecure/TPAGECTL_INSECURE config) that skips TLS certificate verification. Prints a warning to stderr: warning: TLS certificate verification disabled (--insecure).
--ca-cert (#259) — new root flag (and ca_cert/TPAGECTL_CA_CERT config) accepting a path to a PEM CA bundle. The bundle is appended to the system pool via x509.SystemCertPool(); a file with no certificates, or an unreadable file, errors before any request is made.
System CA pool default (#260) — when no controls are configured the client keeps its default transport, which verifies against the system CA pool. No behavior change for existing users.

Why

Roadmap task #261, the last item in Phase 6.

Testing

Client: NewTLSConfig none/insecure/valid-CA (self-signed PEM)/no-certs/missing-file; WithTLSConfig wires the transport; default New keeps the nil transport (system pool)
End-to-end vs httptest.NewTLSServer: --insecure connects and warns; --ca-cert <server cert> verifies without disabling verification; plain status fails on an untrusted cert (proves the system-pool default)
Flag wiring reaches resolved config; nonexistent --ca-cert errors early
go test -race ./... passes (25 packages)
golangci-lint run clean
pre-commit hooks pass (gofmt, go mod tidy, go test)

Breaking Changes

None.

Notes

New client.NewTLSConfig(insecure, caCertFile) (*tls.Config, error) and client.WithTLSConfig(*tls.Config) option; newClient applies them in cmd/tpagectl/main.go. --insecure warning goes to stderr so stdout stays scriptable.

Closes #258
Closes #259
Closes #260
Closes #261

## What Implements Phase 6 TLS verification controls (roadmap #261). - **`--insecure` (#258)** — new root flag (and `insecure`/`TPAGECTL_INSECURE` config) that skips TLS certificate verification. Prints a warning to stderr: `warning: TLS certificate verification disabled (--insecure)`. - **`--ca-cert` (#259)** — new root flag (and `ca_cert`/`TPAGECTL_CA_CERT` config) accepting a path to a PEM CA bundle. The bundle is appended to the system pool via `x509.SystemCertPool()`; a file with no certificates, or an unreadable file, errors before any request is made. - **System CA pool default (#260)** — when no controls are configured the client keeps its default transport, which verifies against the system CA pool. No behavior change for existing users. ## Why Roadmap task #261, the last item in Phase 6. ## Testing - [x] Client: `NewTLSConfig` none/insecure/valid-CA (self-signed PEM)/no-certs/missing-file; `WithTLSConfig` wires the transport; default `New` keeps the nil transport (system pool) - [x] End-to-end vs `httptest.NewTLSServer`: `--insecure` connects and warns; `--ca-cert <server cert>` verifies without disabling verification; plain `status` fails on an untrusted cert (proves the system-pool default) - [x] Flag wiring reaches resolved config; nonexistent `--ca-cert` errors early - [x] `go test -race ./...` passes (25 packages) - [x] `golangci-lint run` clean - [x] pre-commit hooks pass (gofmt, go mod tidy, go test) ## Breaking Changes None. ## Notes New `client.NewTLSConfig(insecure, caCertFile) (*tls.Config, error)` and `client.WithTLSConfig(*tls.Config)` option; `newClient` applies them in `cmd/tpagectl/main.go`. `--insecure` warning goes to stderr so stdout stays scriptable. Closes #258 Closes #259 Closes #260 Closes #261

automation added 1 commit

2026-08-08 05:06:23 +00:00

feat(tpagectl): add TLS verification controls

Test and Release / lint (pull_request) Successful in 7m13s

Details

Test and Release / test (pull_request) Successful in 7m29s

Details

ac623b9da4

Add client.NewTLSConfig and WithTLSConfig to build a transport with the
system CA pool plus an optional custom PEM bundle, or with verification
disabled. Default behavior is unchanged: no controls means the default
transport and system CA pool.

Wire the root --insecure and --ca-cert flags (also configurable as
insecure/ca_cert and TPAGECTL_INSECURE/TPAGECTL_CA_CERT), with a stderr
warning when verification is disabled.

Closes #258
Closes #259
Closes #260
Closes #261

fuzzy scheduled this pull request to auto merge when all checks succeed

2026-08-08 05:06:41 +00:00

the.auditor referenced this pull request

2026-08-08 05:09:27 +00:00

client.WithTLSConfig replaces transport, losing proxy support and default timeouts #598

the.auditor referenced this pull request

2026-08-08 05:09:28 +00:00

NewTLSConfig parses CA bundle even when insecure is set #599

the.auditor approved these changes

2026-08-08 05:09:37 +00:00

the.auditor left a comment

Thorough implementation of the TLS verification controls. NewTLSConfig handles the none/insecure/custom-CA cases cleanly, the CA bundle is appended to the system pool, missing/empty bundles fail before any request, the warning goes to stderr, and the e2e tests (--insecure connects and warns, --ca-cert verifies, default rejects untrusted certs) prove the system-pool default. Approving; two follow-up suggestions filed.

Suggestions

internal/tpagectl/client/client.go:60 – WithTLSConfig replaces the transport with a bare &http.Transport{}, dropping http.DefaultTransport behavior: ProxyFromEnvironment (so HTTP_PROXY/HTTPS_PROXY/NO_PROXY are ignored whenever --insecure or --ca-cert is set), default dial/handshake/idle timeouts, and HTTP/2. Clone http.DefaultTransport and set TLSClientConfig on the clone. Filed as #598.
internal/tpagectl/client/client.go:145 – NewTLSConfig reads and validates the CA bundle even when insecure is set, so --insecure --ca-cert <bad-path> errors out despite verification being disabled. Skip CA handling when insecure, or document the flags as mutually exclusive. Filed as #599.

Question

The --insecure warning is printed whenever Insecure is true, including when it comes from config/env — the message references (--insecure) which may mislead when the flag wasn't passed. Also, CA-file errors raised in the root Before hook abort all commands (same pattern noted in #596). Consider resolving TLS config lazily for commands that need it.

Thorough implementation of the TLS verification controls. `NewTLSConfig` handles the none/insecure/custom-CA cases cleanly, the CA bundle is appended to the system pool, missing/empty bundles fail before any request, the warning goes to stderr, and the e2e tests (`--insecure` connects and warns, `--ca-cert` verifies, default rejects untrusted certs) prove the system-pool default. Approving; two follow-up suggestions filed. ## Suggestions 1. `internal/tpagectl/client/client.go:60` – `WithTLSConfig` replaces the transport with a bare `&http.Transport{}`, dropping `http.DefaultTransport` behavior: `ProxyFromEnvironment` (so `HTTP_PROXY`/`HTTPS_PROXY`/`NO_PROXY` are ignored whenever `--insecure` or `--ca-cert` is set), default dial/handshake/idle timeouts, and HTTP/2. Clone `http.DefaultTransport` and set `TLSClientConfig` on the clone. Filed as #598. 2. `internal/tpagectl/client/client.go:145` – `NewTLSConfig` reads and validates the CA bundle even when `insecure` is set, so `--insecure --ca-cert <bad-path>` errors out despite verification being disabled. Skip CA handling when insecure, or document the flags as mutually exclusive. Filed as #599. ## Question - The `--insecure` warning is printed whenever `Insecure` is true, including when it comes from config/env — the message references `(--insecure)` which may mislead when the flag wasn't passed. Also, CA-file errors raised in the root `Before` hook abort all commands (same pattern noted in #596). Consider resolving TLS config lazily for commands that need it.

fuzzy merged commit ac623b9da4 into main

2026-08-08 05:09:55 +00:00

fuzzy deleted branch feat/cli-tls-controls

2026-08-08 05:09:55 +00:00