refactor(httpclient): unify HTTP client across all platforms by frjcomp · Pull Request #646 · CompassSecurity/pipeleek

frjcomp · 2026-06-01T11:59:36Z

Summary

Centralises all HTTP client configuration in pkg/httpclient so every platform (Bitbucket, DevOps, Circle, Jenkins, Gitea) shares the same TLS, proxy, timeout, and retry settings — previously each used its own ad-hoc configuration.

Changes

`pkg/httpclient/client.go` — central factory

SetInsecureSkipVerify(bool) — toggleable TLS verification (default true, preserving existing behaviour)
SetSOCKSProxy(string) — SOCKS5 proxy support via golang.org/x/net/proxy
SetHTTPTimeout(duration) — configurable per-request timeout
GetPipeleekTransport() — new helper returning a configured *http.Transport for non-retryable clients (Resty)
Thread-safe global config via sync.RWMutex

`internal/cmd/root.go` — three new persistent flags

Flag	Default	Description
`--insecure-skip-verify`	`true`	Skip TLS certificate verification
`--socks-proxy`	`""`	SOCKS5 proxy URL
`--http-timeout`	`0`	HTTP request timeout (0 = no timeout)

Platform injection

Platform	Before	After
Bitbucket (Resty)	plain `resty.New()`	`.SetTransport(GetPipeleekTransport())`
Azure DevOps (Resty)	plain `resty.New()`	`.SetTransport(GetPipeleekTransport())`
CircleCI	`&http.Client{Timeout: 45s}`	`GetPipeleekHTTPClient().StandardClient()`
Jenkins	`nil` (SDK default)	`GetPipeleekHTTPClient().StandardClient()`
Gitea SDK	post-hoc transport mutation	`gitea.SetHTTPClient(GetPipeleekHTTPClient().StandardClient())` — clean injection, no mutation

Note: The GitHub SDK client uses a dedicated go-github-ratelimit transport and is intentionally left unchanged.

Tests

Expanded pkg/httpclient/client_test.go — 22 tests covering all new setter/getter functions
New pkg/bitbucket/scan/client_test.go — 6 tests (transport injection, cookie jar, TLS config)
New pkg/devops/scan/client_test.go — 4 tests (transport injection, URLs, TLS config)
New pkg/gitea/scan/scanner_test.go — 6 tests using httptest.Server mock (SDK injection, auth headers, cookie jar, no transport mutation, TLS config, invalid URL)

Docs

docs/introduction/configuration.md — new "HTTP Client Settings" section documenting all four global flags

Test results

Unit suite (go test ./... -race): all packages pass ✅
E2E (Jenkins, Bitbucket, Circle): all pass ✅

- Introduce configurable global state in pkg/httpclient: - SetInsecureSkipVerify(bool) — toggleable TLS verification (default true) - SetSOCKSProxy(string) — SOCKS5 proxy support via golang.org/x/net/proxy - SetHTTPTimeout(duration) — per-request timeout - GetPipeleekTransport() — shared *http.Transport for non-retryable clients - Wire three new persistent root flags: --insecure-skip-verify (default true), --socks-proxy, --http-timeout - Inject Pipeleek transport/client into all platforms: - Bitbucket/DevOps (Resty): .SetTransport(GetPipeleekTransport()) - Circle: GetPipeleekHTTPClient().StandardClient() - Jenkins: GetPipeleekHTTPClient().StandardClient() (was nil) - Gitea SDK: gitea.SetHTTPClient(GetPipeleekHTTPClient().StandardClient()) removing post-hoc transport mutation - Add unit tests for new httpclient functions (22 tests) - Add unit tests for Bitbucket NewClient, DevOps NewClient, and Gitea InitializeOptions (16 new tests with httptest mock servers) - Update docs/introduction/configuration.md with HTTP Client Settings section

Copilot

Pull request overview

This PR centralizes HTTP client/transport configuration in pkg/httpclient and injects it into multiple platform scanners/SDK clients so they share consistent TLS, proxy/SOCKS, and timeout behavior, with new global CLI flags to control these settings.

Changes:

Added a global, mutex-protected HTTP client configuration (TLS verify toggle, SOCKS proxy, request timeout) and transport factory helpers in pkg/httpclient.
Updated platform clients (Bitbucket, Azure DevOps, CircleCI, Jenkins, Gitea SDK) to use the shared Pipeleek transport / standard client injection.
Added root-level persistent CLI flags and documented the new HTTP client settings.

Reviewed changes

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

Show a summary per file

File	Description
`pkg/httpclient/client.go`	Introduces global HTTP client config and transport/client factories (TLS/SOCKS/proxy/timeout).
`pkg/httpclient/client_test.go`	Adds unit tests for the new global setters and transport behavior.
`internal/cmd/root.go`	Adds persistent flags and wires them into `pkg/httpclient` during `PersistentPreRun`.
`pkg/bitbucket/scan/api.go`	Injects Pipeleek transport into Resty client.
`pkg/bitbucket/scan/client_test.go`	Adds tests validating transport injection, cookie jar behavior, and TLS settings.
`pkg/devops/scan/api.go`	Injects Pipeleek transport into Resty client.
`pkg/devops/scan/client_test.go`	Adds tests validating transport injection and TLS settings.
`pkg/circle/scan/scanner.go`	Switches CircleCI API client to Pipeleek’s standard HTTP client.
`pkg/jenkins/scan/client.go`	Injects Pipeleek’s standard HTTP client into the Jenkins SDK client.
`pkg/gitea/scan/scanner.go`	Uses Pipeleek clients and cleanly injects the standard client into the Gitea SDK.
`pkg/gitea/scan/scanner_test.go`	Adds tests validating SDK injection, auth header behavior, cookie jar, and TLS settings.
`docs/introduction/configuration.md`	Documents new global HTTP client flags and examples.

- Set Timeout on SOCKS net.Dialer to prevent indefinite hangs on unreachable proxies; uses configured --http-timeout or 30s fallback - Correct docs: --http-timeout only applies to retryable HTTP client users (GitLab, Gitea, Jenkins, CircleCI, NIST); Bitbucket and Azure DevOps use Resty transport injection and are unaffected

…ll platforms - GetPipeleekHTTPClient now returns *resty.Client (was *retryablehttp.Client) - All platform scan/util packages updated to use native Resty API: .R().Get()/.Post(), resp.StatusCode(), resp.Bytes(), client.Client() - Platforms using SDK clients (GitLab, Gitea, gojenkins, CircleCI) call .Client() to extract *http.Client while still inheriting the shared transport - Removes direct dependency on go-retryablehttp; retained as indirect (transitive via TruffleHog) - Update docs/introduction/proxying.md: fix stale retryablehttp reference in http-timeout scope note - Add pipeleek_test_build to .gitignore

… GitLab enum client - github/scan: pass httpclient.GetPipeleekTransport() as base to github_ratelimit.New() instead of nil (which fell back to http.DefaultTransport, bypassing TLS/proxy config) - gitlab/enum: replace bare resty.New() with httpclient.GetPipeleekHTTPClient() so --proxy, --tls-verification and --http-timeout flags apply to enum calls

…h GetPipeleekHTTPClient Both clients were using resty.New().SetTransport(GetPipeleekTransport()) which missed retry logic (count, wait, conditions) and --http-timeout. Switching to GetPipeleekHTTPClient() restores full parity with other platforms.

Replace manual AddRetryConditions (429 / 5xx-except-501 / error logic) with EnableRetryDefaultConditions(), which implements the same behaviour including 501 exclusion, status-0, and transient URL errors. Logging is preserved via AddRetryHooks. Update test to assert the new configuration style.

- scanner/rules: replace removed retryablehttp RetryMax field with resty SetRetryCount(0) in TestDownloadFile_HTTPError - gitlab/container/artipacked: fatal on failed project iteration so the command exits non-zero when the GitLab instance is unreachable (regression: go-retryablehttp used to hang on retries long enough for the 10s test timeout to kill the process; Resty+plain transport exits immediately and silently, masking the error) - formatting: gofmt/alignment fixes in root.go, configuration.md, proxying.md, http_utils_test.go, enum.go, users/enum.go, nist_test.go

'example.invalid' fails DNS lookup immediately (NXDOMAIN is not a temporary error so Resty does not retry), causing the command to exit in <1s instead of timing out at 3s. Replace with 192.0.2.5 as the hostname value — same TEST-NET-1 range used by the other shodan tests — so the command hangs on the TCP connect and the 3s test timeout fires.

Copilot

Pull request overview

Copilot reviewed 36 out of 37 changed files in this pull request and generated 5 comments.

- Register missing --tls-verification persistent flag in root.go (was wired to httpclient but never exposed to CLI users after rename from --insecure-skip-verify) - Restore CircleCI 45s fallback timeout: GetPipeleekHTTPClient returns a Resty client with Timeout=0 by default; add explicit fallback so the CircleCI API client preserves the original 45s timeout behaviour - Fix scanNamespace error handling in artipacked/scanner.go to use log.Fatal() consistent with fetchProjects (was log.Error()+return, which silently continued on network failure) - Update proxying.md: --http-timeout now applies to all platforms including Bitbucket and Azure DevOps; clarify GitHub SDK exception

- pkg/devops/scan: Azure DevOps is cloud-only (dev.azure.com) and always presents a valid certificate. Override the transport TLS config with SetTLSClientConfig to enforce verification (MinVersion TLS 1.2, InsecureSkipVerify=false) regardless of the global --tls-verification flag. Update TestAzureDevOpsNewClient to assert this invariant. - pkg/gitea/scan: Fix stale comment that incorrectly attributed header injection to HeaderRoundTripper; headers are now set via Resty's SetHeaders. Note: go-retryablehttp remains in go.mod as // indirect because gitlab.com/gitlab-org/api/client-go depends on it transitively. There are zero direct imports of retryablehttp in the codebase.

Copilot

Pull request overview

Copilot reviewed 36 out of 37 changed files in this pull request and generated 6 comments.

+	// #nosec G402 - InsecureSkipVerify is user-configurable; defaults to true so that
+	// scanning tools can reach self-hosted instances with self-signed certificates.
+	tr := &http.Transport{
+		TLSClientConfig: &tls.Config{InsecureSkipVerify: cfg.insecureSkipVerify},
+	}


 		baseURL = "https://api.github.com/"
 	}
-	rateLimiter := github_ratelimit.New(nil,
+	rateLimiter := github_ratelimit.New(httpclient.GetPipeleekTransport(),


 	if err != nil {
-		log.Error().Stack().Err(err).Msg("Failed iterating group projects")
-		return
+		log.Fatal().Stack().Err(err).Msg("Failed iterating group projects")
 	}


 	if err != nil {
-		log.Error().Stack().Err(err).Msg("Failed iterating projects")
-		return
+		log.Fatal().Stack().Err(err).Msg("Failed iterating projects")
 	}


+	rootCmd.PersistentFlags().BoolVar(&TLSVerification, "tls-verification", false, "Enable TLS certificate verification (by default verification is skipped to support self-signed certificates)")
 	rootCmd.PersistentFlags().BoolVar(&IgnoreProxy, "ignore-proxy", false, "Ignore HTTP_PROXY environment variable")
+	rootCmd.PersistentFlags().StringVar(&Proxy, "proxy", "", "Proxy URL, e.g. http://127.0.0.1:8080 or socks5://127.0.0.1:1080 (takes precedence over HTTP_PROXY)")
+	rootCmd.PersistentFlags().DurationVar(&HTTPTimeout, "http-timeout", 0, "HTTP request timeout, e.g. 30s or 2m (default: no timeout)")


 			httpclient.SetIgnoreProxy(IgnoreProxy)
+			httpclient.SetInsecureSkipVerify(!TLSVerification)
+			httpclient.SetProxy(Proxy)
+			httpclient.SetHTTPTimeout(HTTPTimeout)


Copilot AI review requested due to automatic review settings June 1, 2026 11:59

Copilot started reviewing on behalf of frjcomp June 1, 2026 11:59 View session

Copilot AI reviewed Jun 1, 2026

View reviewed changes

Comment thread pkg/httpclient/client.go Outdated

Comment thread docs/introduction/configuration.md Outdated

vscode added 7 commits June 2, 2026 06:43

frjcomp requested a review from Copilot June 24, 2026 12:46

Copilot started reviewing on behalf of frjcomp June 24, 2026 12:46 View session

Copilot AI reviewed Jun 24, 2026

View reviewed changes

Comment thread internal/cmd/root.go

Comment thread docs/introduction/proxying.md Outdated

Comment thread docs/introduction/proxying.md Outdated

Comment thread pkg/gitlab/container/artipacked/scanner.go

Comment thread pkg/circle/scan/scanner.go

vscode added 2 commits June 24, 2026 13:12

frjcomp requested a review from Copilot June 24, 2026 14:04

Copilot started reviewing on behalf of frjcomp June 24, 2026 14:05 View session

Copilot AI reviewed Jun 24, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor(httpclient): unify HTTP client across all platforms#646

refactor(httpclient): unify HTTP client across all platforms#646
frjcomp wants to merge 10 commits into
mainfrom
refactor/http-client-unification

frjcomp commented Jun 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

frjcomp commented Jun 1, 2026

Summary

Changes

pkg/httpclient/client.go — central factory

internal/cmd/root.go — three new persistent flags

Platform injection

Tests

Docs

Test results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

`pkg/httpclient/client.go` — central factory

`internal/cmd/root.go` — three new persistent flags