Skip to content

feat: add --insecure to test cmd#375

Merged
canercidam merged 2 commits intomainfrom
fryd/test-add-insecure-option
Feb 18, 2026
Merged

feat: add --insecure to test cmd#375
canercidam merged 2 commits intomainfrom
fryd/test-add-insecure-option

Conversation

@fkondej
Copy link
Contributor

@fkondej fkondej commented Feb 18, 2026

No description provided.

@fkondej fkondej self-assigned this Feb 18, 2026
@fkondej fkondej requested a review from canercidam as a code owner February 18, 2026 14:55
@claude
Copy link
Contributor

claude bot commented Feb 18, 2026

Claude finished @fkondej's task in 1m 5s —— View job


PR Review: feat: add --insecure to test cmd

  • Read changed files and PR diff
  • Review code quality, bugs, security, and performance
  • Post review feedback

Summary

This PR adds an --insecure flag to the test command to skip TLS certificate verification, useful for self-signed certs. The implementation is clean and contained.

Feedback

No blocking issues. One minor suggestion left as an inline comment:

  • Linter annotation (playground/test_tx.go:81): The InsecureSkipVerify: true will be flagged by gosec/staticcheck. Adding a //nolint:gosec comment documents that it's intentional and user-gated.

The dialRPC helper function is well-structured — it avoids code duplication across the two dial sites and correctly scopes the insecure transport only when the flag is set.

if cfg.Insecure {
httpClient := &http.Client{
Transport: &http.Transport{
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: consider adding a //nolint:gosec or // #nosec G402 comment on the InsecureSkipVerify: true line. Static analysis tools (gosec, staticcheck) will flag this. Adding the annotation makes it clear this is intentional and user-gated behind --insecure.

Suggested change
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec // user-requested via --insecure flag

@claude
Copy link
Contributor

claude bot commented Feb 18, 2026

Claude finished @canercidam's task in 1m 32s —— View job


PR Review: feat: add --insecure to test cmd

  • Read changed files and PR diff
  • Review code quality, bugs, security, and performance
  • Post review feedback

Summary

This PR adds an --insecure flag to the test command to skip TLS certificate verification, useful for self-signed certs. The implementation is clean — the dialRPC helper avoids duplication across both dial sites and correctly scopes the insecure transport only when the flag is set.

Issues

Linter will fail: gosec is enabled in .golangci.yaml (G402 is not excluded). The InsecureSkipVerify: true at playground/test_tx.go:81 needs a //nolint:gosec directive — left as an inline suggestion.

No other issues found. The HTTPS hint logic in main.go handles the empty --el-rpc default correctly (empty string parses to nil scheme).

if cfg.Insecure {
httpClient := &http.Client{
Transport: &http.Transport{
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
Copy link
Contributor

Choose a reason for hiding this comment

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

gosec is enabled in .golangci.yaml and G402 is not excluded, so this line will fail CI. Add a nolint directive:

Suggested change
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec // user-requested via --insecure flag

@canercidam canercidam merged commit 8db4bcc into main Feb 18, 2026
17 checks passed
@canercidam canercidam deleted the fryd/test-add-insecure-option branch February 18, 2026 16:24
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