Skip to content

Conversation

@dwisiswant0
Copy link
Member

@dwisiswant0 dwisiswant0 commented Apr 13, 2025

Summary by CodeRabbit

  • New Features

    • Certificate Transparency (CT) logs streaming mode with new CLI flags; auto-enables when no input is provided.
    • SOCKS5 proxy support via a new flag.
    • Output enriched with CT log metadata; SAN enabled by default in CT mode.
  • Documentation

    • README updated with CT streaming usage/examples, new proxy flag, and Go 1.24 requirement.
    • Added contributor guide (CLAUDE.md).
  • Chores

    • Upgraded to Go 1.24, refreshed dependencies, updated Docker base image.
    • Added vendor/ to .gitignore.
    • Updated displayed version banner.
  • Bug Fixes

    • Improved stability via stricter resource cleanup and error handling.

@dwisiswant0 dwisiswant0 requested a review from ehsandeep April 13, 2025 15:56
@coderabbitai
Copy link

coderabbitai bot commented Apr 13, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Adds Certificate Transparency (CT) logs streaming feature, new ctlogs package, CLI flags and defaults, runner execution path, proxy support, and response fields for CT metadata. Updates Go/Docker versions and dependencies. Improves cleanup/error handling, testing, and docs (including CT streaming usage). Minor output and ignore rule tweaks.

Changes

Cohort / File(s) Summary
Documentation
README.md, CLAUDE.md, .gitignore
Added CT Logs Streaming docs and new flag (-proxy); installation Go version updated to 1.24; added contributor guide; ignore vendor/.
Build & Dependencies
Dockerfile, go.mod
Builder base updated to Go 1.24-alpine; module Go version to 1.24.0; added CT and bloom filter deps; multiple direct/indirect upgrades.
CLI & Entry
cmd/tlsx/main.go
Introduces CT logs flags (-ct-logs/-ctl, -ctl-beginning, -ctl-index), default-to-CT when no input/STDIN, SOCKS5 proxy flag, stdin detection, validation tweaks, health-check exit path.
Runner Core
internal/runner/runner.go, internal/runner/banner.go, internal/runner/healthcheck.go, internal/runner/runner_test.go
New CT logs execution flow; per-worker client creation; proxy dialer integration; stdin guard; default CT behavior; banner version bump; explicit Close() error handling; comprehensive CT tests.
CT Logs Service (new)
pkg/ctlogs/*
New CT streaming service, options, client with backoff, utils, stats; certificate-to-response mappers; inverse bloom dedupe; tests for backoff, utils, service.
Examples
examples/ctlogs/main.go
New example demonstrating CT streaming and first-cert print.
TLS Client Options & Response
pkg/tlsx/clients/clients.go, pkg/tlsx/clients/utils.go, pkg/tlsx/clients/clients_test.go
Added proxy and CT flags to Options; CT metadata fields to Response; updated self-signed logic to consider SANs; added tests.
TLS Backends Cleanup
pkg/tlsx/tls/tls.go, pkg/tlsx/tls/tls_test.go, pkg/tlsx/ztls/ztls.go, pkg/tlsx/ztls/utils.go, pkg/tlsx/ztls/ztls_test.go, pkg/tlsx/openssl/openssl.go, pkg/tlsx/openssl/openssl_test.go
Standardized defer Close() to ignore errors; updated self-signed calls with SANs; tests explicitly discard write returns.
Auxiliary Commands
cmd/update-cipherstatus/main.go, cmd/update-rootcerts/main.go
Defer Close() now logs/ignores errors via wrappers.
Output Handling
pkg/output/file_writer.go, pkg/output/output.go
Propagate Flush() errors on Close; minor formatting and string replace update.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant CLI as cmd/tlsx (CLI)
  participant Runner as internal/runner.Runner
  participant CT as pkg/ctlogs.CTLogsService
  participant Log as CT Log API
  participant Out as Output Writer

  User->>CLI: tlsx (no -u/-l/STDIN)
  CLI->>Runner: options{ CTLogs=default, proxy?, flags }
  Runner->>CT: New(opts) + Start()
  par Stream per log
    CT->>Log: GetSTH / GetEntries (polling)
    Log-->>CT: Entries (batch)
    loop For each entry
      CT->>CT: Deduplicate (inverse BF)
      alt unique
        CT->>Runner: Callback(meta, cert)
        Runner->>Out: Write Response (CTLogSource/Index/TreeSize/Lag)
      else duplicate
        CT->>CT: Update stats (duplicate)
      end
    end
  and Stop condition
    User-->>CLI: interrupt/exit
    CLI->>Runner: Stop
    Runner->>CT: Stop()
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

I twitch my ears at logs that gleam,
A billion leaves within the stream.
I nibble dupes—discard, proceed,
Hop CT paths with jittered speed.
With proxy burrow, ports in sight,
I print the CN—then bound to night. 🐇🔒🌙


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3e4f27a and e8f0c1b.

⛔ Files ignored due to path filters (9)
  • .github/dependabot.yml is excluded by !**/*.yml
  • .github/workflows/build-test.yml is excluded by !**/*.yml
  • .github/workflows/lint-test.yml is excluded by !**/*.yml
  • .github/workflows/release-binary.yml is excluded by !**/*.yml
  • .github/workflows/release-test.yml is excluded by !**/*.yml
  • .github/workflows/update-cipherstatus.yml is excluded by !**/*.yml
  • .github/workflows/update-rootca.yml is excluded by !**/*.yml
  • assets/cipherstatus_data.json is excluded by !**/*.json
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (33)
  • .gitignore (1 hunks)
  • CLAUDE.md (1 hunks)
  • Dockerfile (1 hunks)
  • README.md (4 hunks)
  • cmd/tlsx/main.go (6 hunks)
  • cmd/update-cipherstatus/main.go (1 hunks)
  • cmd/update-rootcerts/main.go (1 hunks)
  • examples/ctlogs/main.go (1 hunks)
  • go.mod (3 hunks)
  • internal/runner/banner.go (2 hunks)
  • internal/runner/healthcheck.go (2 hunks)
  • internal/runner/runner.go (7 hunks)
  • internal/runner/runner_test.go (2 hunks)
  • pkg/ctlogs/backoff.go (1 hunks)
  • pkg/ctlogs/backoff_test.go (1 hunks)
  • pkg/ctlogs/client.go (1 hunks)
  • pkg/ctlogs/ctlogs.go (1 hunks)
  • pkg/ctlogs/ctlogs_test.go (1 hunks)
  • pkg/ctlogs/options.go (1 hunks)
  • pkg/ctlogs/service_test.go (1 hunks)
  • pkg/ctlogs/utils.go (1 hunks)
  • pkg/output/file_writer.go (1 hunks)
  • pkg/output/output.go (2 hunks)
  • pkg/tlsx/clients/clients.go (5 hunks)
  • pkg/tlsx/clients/clients_test.go (2 hunks)
  • pkg/tlsx/clients/utils.go (1 hunks)
  • pkg/tlsx/openssl/openssl.go (1 hunks)
  • pkg/tlsx/openssl/openssl_test.go (1 hunks)
  • pkg/tlsx/tls/tls.go (2 hunks)
  • pkg/tlsx/tls/tls_test.go (1 hunks)
  • pkg/tlsx/ztls/utils.go (1 hunks)
  • pkg/tlsx/ztls/ztls.go (3 hunks)
  • pkg/tlsx/ztls/ztls_test.go (1 hunks)
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dwisiswant0/ci/fix-forbidden-cipherstatus-and-root-CA-updates

Comment @coderabbitai help to get the list of available commands and usage tips.

@Mzack9999 Mzack9999 self-requested a review September 12, 2025 10:37
@Mzack9999
Copy link
Member

@coderabbitai make an issue for this

@coderabbitai
Copy link

coderabbitai bot commented Sep 12, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

I need to understand what specific issue you'd like me to create. Let me first examine the PR content to get more context.


🏁 Script executed:

#!/bin/bash
# Get PR details and recent commits to understand what might need an issue
gh pr view 801 --repo projectdiscovery/tlsx

# Also check if there are any recent commits or changes that might indicate what needs an issue
gh pr diff 801 --repo projectdiscovery/tlsx | head -50

Length of output: 1891


Now let me see the full diff to understand all the changes being made:


🏁 Script executed:

#!/bin/bash
# Get the complete diff to understand all changes
gh pr diff 801 --repo projectdiscovery/tlsx

# Also check if there are any comments or discussion in the PR that might indicate what issue is needed
gh pr view 801 --repo projectdiscovery/tlsx --comments

Length of output: 25781


@Mzack9999 I can help you create an issue, but I need clarification on what specific issue you'd like me to create. Looking at this PR about fixing CI workflows for cipher status and root CA updates, are you wanting me to create an issue for:

  1. Follow-up improvements to the CI workflows?
  2. The fact that these workflow files are excluded from CodeRabbit reviews due to path filters?
  3. Something else specific about this PR?

Could you please specify what the issue should be about and any particular details you'd like included?

@Mzack9999
Copy link
Member

@coderabbitai make an issue for the PR related to GitHub actions simplification and improvement related to ciphers

@coderabbitai
Copy link

coderabbitai bot commented Sep 12, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

I'll create an issue for GitHub Actions simplification and improvement related to ciphers based on this PR.

@Mzack9999 I've created an issue for GitHub Actions simplification and improvement related to ciphers. The issue captures the context from PR #801 and outlines potential areas for improvement in cipher-related workflows, including simplification opportunities, optimization possibilities, and maintenance enhancements.


🏁 Script executed:

#!/bin/bash
# Create a GitHub issue for cipher-related GitHub Actions improvements
gh issue create \
  --repo projectdiscovery/tlsx \
  --title "Improve and simplify GitHub Actions workflows for cipher testing and validation" \
  --body "## Description

This issue tracks potential improvements and simplifications to our GitHub Actions workflows related to cipher testing and validation, following the work done in PR #801.

## Context

PR #801 addressed fixes for forbidden cipher status and root CA updates in our CI workflows. This highlighted opportunities for further improvements to make our cipher-related workflows more efficient and maintainable.

## Potential Improvements

- [ ] Simplify cipher testing workflows to reduce complexity
- [ ] Optimize cipher validation processes in CI
- [ ] Consolidate redundant cipher-related workflow steps
- [ ] Improve error handling and reporting for cipher status checks
- [ ] Enhance root CA update mechanisms in workflows
- [ ] Review and optimize workflow triggers for cipher-related changes

## References

- Related PR: https://github.com/projectdiscovery/tlsx/pull/801
- Requested by: @Mzack9999

## Acceptance Criteria

- GitHub Actions workflows for cipher testing are simplified and more maintainable
- Cipher validation processes are optimized for performance
- Documentation is updated to reflect workflow changes
- Testing coverage for cipher-related functionality is maintained or improved

/cc @Mzack9999" \
  --assignee Mzack9999

Length of output: 51


@Mzack9999 Perfect! I've successfully created issue #866 for GitHub Actions simplification and improvement related to ciphers. The issue is now available at #866 and has been assigned to you.

The issue includes:

@Mzack9999 Mzack9999 changed the base branch from main to dev September 12, 2025 13:00
@Mzack9999 Mzack9999 merged commit d86b8cf into dev Sep 12, 2025
6 of 7 checks passed
@Mzack9999 Mzack9999 deleted the dwisiswant0/ci/fix-forbidden-cipherstatus-and-root-CA-updates branch September 12, 2025 13:01
@Mzack9999 Mzack9999 linked an issue Sep 15, 2025 that may be closed by this pull request
6 tasks
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.

Improve and simplify GitHub Actions workflows for cipher testing and validation

5 participants