Skip to content

fix skip-verify not applying to thin event forwarding - #1534

Merged
bernerd-stripe merged 3 commits into
stripe:masterfrom
knQzx:fix-skip-verify-thin-events
Apr 6, 2026
Merged

bernerd-stripe merged 3 commits into
stripe:masterfrom
knQzx:fix-skip-verify-thin-events

Conversation

@knQzx

@knQzx knQzx commented Apr 4, 2026

Copy link
Copy Markdown
Contributor

summary

  • PostV2 (thin event forwarding) was using http.DefaultClient instead of the configured HTTP client, so the --skip-verify flag had no effect on thin events sent via --forward-thin-to / --forward-thin-connect-to
  • changed PostV2 to use c.cfg.HTTPClient (same as Post does for v1 events), which carries the InsecureSkipVerify TLS config
  • added a test that confirms PostV2 uses the configured client by hitting an HTTPS test server with a self-signed cert

test plan

  • ran all existing proxy tests - all pass
  • added TestPostV2_UsesConfiguredHTTPClient which creates an HTTPS test server and verifies PostV2 succeeds with InsecureSkipVerify: true (would fail with x509 error using http.DefaultClient)

fixes #1434

PostV2 was using http.DefaultClient instead of the configured
HTTP client, so the InsecureSkipVerify TLS setting from --skip-verify
was being ignored for thin event forwarding.
@knQzx
knQzx requested a review from a team as a code owner April 4, 2026 22:55
@cla-assistant

cla-assistant Bot commented Apr 4, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@cla-assistant

cla-assistant Bot commented Apr 4, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

1 similar comment
@cla-assistant

cla-assistant Bot commented Apr 4, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@bernerd-stripe bernerd-stripe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👋 Hi @knQzx, thanks for the contribution (and the test coverage).

I think the test can be cleaned up a bit by using the ts.Client() helper for obtaining an HTTP client that's pre-configured to access the test server, so I left a suggestion to replace that (and remove the unused dependencies).

Comment thread pkg/proxy/endpoint_test.go Resolved
Comment thread pkg/proxy/endpoint_test.go Resolved
Comment thread pkg/proxy/endpoint_test.go Resolved
@knQzx

knQzx commented Apr 6, 2026

Copy link
Copy Markdown
Contributor Author

sorry about that, simplified the test to use ts.Client() and removed the unused imports. thanks for the feedback!

@knQzx
knQzx requested a review from bernerd-stripe April 6, 2026 17:05
@bernerd-stripe
bernerd-stripe merged commit c2b3dc7 into stripe:master Apr 6, 2026
7 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.

Bug: --skip-verify flag does not apply to thin event forwarding

2 participants