Skip to content

feat(certinfo): add tls-info flag to display negotiated TLS settings …#34

Merged
xenOs76 merged 2 commits into
mainfrom
feat/certinfo_tls_scan
May 18, 2026
Merged

feat(certinfo): add tls-info flag to display negotiated TLS settings …#34
xenOs76 merged 2 commits into
mainfrom
feat/certinfo_tls_scan

Conversation

@xenOs76
Copy link
Copy Markdown
Owner

@xenOs76 xenOs76 commented May 18, 2026

…and supported protocols and ciphers

Summary by CodeRabbit

  • New Features

    • Added --tls-info flag to display negotiated TLS protocol and cipher information from remote endpoints, with probing of supported protocols and cipher suites.
  • Documentation

    • Updated README with expanded certinfo command examples and documentation for the new --tls-info flag.

Review Change Stack

@xenOs76 xenOs76 self-assigned this May 18, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 18, 2026

Warning

Rate limit exceeded

@xenOs76 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 46 minutes and 20 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 982d3a10-3460-440b-948b-132e7480b1d6

📥 Commits

Reviewing files that changed from the base of the PR and between 6558fff and 58a6300.

📒 Files selected for processing (1)
  • internal/certinfo/certinfo_handlers.go
📝 Walkthrough

Walkthrough

This PR introduces TLS protocol and cipher suite probing for remote endpoints in the certinfo command. It adds a --tls-info flag that triggers scanning and display of negotiated TLS connection details, supported protocol versions, and cipher suite compatibility when provided alongside --tls-endpoint.

Changes

TLS Probing and Negotiation Info Feature

Layer / File(s) Summary
Config data model and chainable setter
internal/certinfo/certinfo.go
Config struct extended with TLSInfoRequested, NegotiatedProtocol, NegotiatedCipher, ProbedProtocols, and ProbedCiphers fields; new ProbedCipher type introduced; SetTLSInfoRequested chainable method added.
TLS probing implementation and rendering
internal/certinfo/certinfo_handlers.go
Imports updated for sorting and concurrency support; GetRemoteCerts captures negotiated protocol/cipher from TLS connection state; PrintData conditionally calls ProbeTLSInfo when enabled; helper functions added for TLS version conversion, protocol probing, and single-cipher probing; ProbeTLSInfo orchestrates protocol/cipher scans; probeCiphersConcurrently worker pool implements concurrent cipher suite testing; printTLSInfo renders negotiated details and scan results in table format.
CLI flag, validation, and wiring
internal/cmd/certinfo.go
New tlsInfo boolean variable and --tls-info flag registered; validation ensures flag requires --tls-endpoint; flag value wired into Config via SetTLSInfoRequested; command help/examples updated with --tls-info usage.
Test fixtures and helper updates
internal/certinfo/main_test.go, internal/certinfo/certinfo_handlers_test.go
generateRSACertificateData creates RSASampleCertBundleFile (combined PEM certificate); test fixture updated to use bundle file; t.Parallel() calls removed from existing tests to prevent parallel execution conflicts.
Unit and integration test coverage
internal/certinfo/certinfo_test.go, internal/cmd/certinfo_test.go
Tests added for ProbeTLSInfo scenarios (enabled, disabled, no endpoint, unreachable); printTLSInfo output validation (with/without ciphers); helper function unit tests (tlsVersionToString, probeCiphersConcurrently); PrintData integration test; CLI command test refactored into focused cases and new TestCertinfoCmd_WithTLSInfo integration test added.
README documentation updates
README.md
Example commands reorganized into "local certificates and keys" and "remote certificates" comment blocks; --tls-info flag added to flags list with description.

Sequence Diagram

sequenceDiagram
  participant PrintData
  participant ProbeTLSInfo
  participant probeProtocol
  participant probeCiphersConcurrently
  participant printTLSInfo
  PrintData->>ProbeTLSInfo: call when TLSInfoRequested
  ProbeTLSInfo->>probeProtocol: test each protocol version
  probeProtocol-->>ProbeTLSInfo: populate ProbedProtocols map
  ProbeTLSInfo->>probeCiphersConcurrently: scan all cipher suites
  probeCiphersConcurrently-->>ProbeTLSInfo: populate ProbedCiphers slice
  PrintData->>printTLSInfo: render negotiated + probed tables
  printTLSInfo-->>PrintData: formatted TLS info output
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • xenOs76/https-wrench#29: Refactors TestCertinfo_PrintData test helpers that are now directly affected by this PR's changes to PrintData (conditional ProbeTLSInfo calls and TLS scan output printing).
  • xenOs76/https-wrench#13: Modifies PrintData and GetRemoteCerts in certinfo_handlers.go; this PR extends those same functions with TLS probing and negotiation capture.
  • xenOs76/https-wrench#30: Refactors the Config type API; this PR then extends that same Config with negotiated TLS fields and probing methods.

Poem

🐰 A wrench that now peeks at ciphers and protocols with grace,
Scanning TLS handshakes across the digital space,
With worker pools bustling and tables so fine,
Your endpoints' secrets? They're practically mine! 🔐

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.73% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly describes the main feature being added: a new tls-info flag to display negotiated TLS settings, which is the primary change throughout the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/certinfo_tls_scan

Warning

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.


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.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/certinfo/certinfo_handlers.go`:
- Around line 325-327: The dialer used for probe dials currently hardcodes
Timeout as 1 * time.Second which can cause false negatives on slow links; change
the net.Dialer Timeout initialization (the variable named dialer) to use the
existing TLSTimeout constant instead of 1 * time.Second, and update the other
identical occurrence later in the file (the second net.Dialer initialization) to
do the same so all probe dials share TLSTimeout.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 081bfcd3-e5bb-4c60-8abc-0986e83ee8f5

📥 Commits

Reviewing files that changed from the base of the PR and between f830bd2 and 6558fff.

📒 Files selected for processing (8)
  • README.md
  • internal/certinfo/certinfo.go
  • internal/certinfo/certinfo_handlers.go
  • internal/certinfo/certinfo_handlers_test.go
  • internal/certinfo/certinfo_test.go
  • internal/certinfo/main_test.go
  • internal/cmd/certinfo.go
  • internal/cmd/certinfo_test.go

Comment thread internal/certinfo/certinfo_handlers.go
@xenOs76
Copy link
Copy Markdown
Owner Author

xenOs76 commented May 18, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 18, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@xenOs76 xenOs76 merged commit 34215c2 into main May 18, 2026
5 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.

1 participant