Skip to content

fix(tls): fix TLS certificate tests to use local certs#626

Merged
fortuna merged 4 commits into
mainfrom
codex/tls-expired-cert-local
Jun 16, 2026
Merged

fix(tls): fix TLS certificate tests to use local certs#626
fortuna merged 4 commits into
mainfrom
codex/tls-expired-cert-local

Conversation

@fortuna

@fortuna fortuna commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Replace the badssl.com-backed TLS certificate error tests with local, runtime-generated certificates.
  • Add a local TLS handshake helper so the tests still exercise NewStreamDialer over TCP.

Root Cause

TestExpired and TestUntrustedRoot depended on live badssl.com endpoints. Those endpoints can reset the TCP connection before Go reaches certificate verification, causing CI flakes unrelated to the code under test.

Validation

  • go test -run 'TestUntrustedRoot|TestExpired' -count=50 -v ./transport/tls
  • go test -race ./transport/tls
  • go test -tags nettest -race -bench '.' ./transport/tls -benchtime=100ms
  • go test -tags nettest -race -bench '.' ./... -benchtime=100ms
  • go test -C x -tags nettest -race -bench '.' ./... -benchtime=100ms

@fortuna fortuna requested review from jyyi1 and ohnorobo June 16, 2026 15:05
@fortuna fortuna marked this pull request as ready for review June 16, 2026 15:05
@fortuna fortuna changed the title [codex] Fix TLS certificate tests to use local certs fix(tls): fix TLS certificate tests to use local certs Jun 16, 2026
@fortuna fortuna enabled auto-merge (squash) June 16, 2026 15:07
@greptile-apps

greptile-apps Bot commented Jun 16, 2026

Copy link
Copy Markdown

Greptile Summary

This PR removes the dependency on live badssl.com endpoints in TestUntrustedRoot and TestExpired by generating certificates at runtime and serving them from a local ephemeral TLS listener, eliminating the CI flakes caused by remote TCP resets before Go reached certificate verification.

  • serveTLSHandshakes spins up a stdTLS.Listen server on 127.0.0.1:0, accepts connections in a goroutine, performs the server-side handshake (necessary so the client's VerifyConnection callback fires), and tears everything down in t.Cleanup.
  • TestUntrustedRoot uses an empty x509.CertPool so the chain is unverifiable → x509.UnknownAuthorityError; TestExpired adds the root CA but uses a leaf with notAfter in the past → x509.CertificateInvalidError{Reason: Expired}.
  • The remaining badssl.com reference in TestRevoked is already skipped via t.Skip so it does not affect CI reliability.

Confidence Score: 4/5

Test-only change that removes external network dependencies; safe to merge.

All changes are confined to test helpers. The cert generation, local TLS server, and cleanup logic are correct. The only note is a minor cleanup-ordering issue in serveTLSHandshakes where a fatal assertion could skip waiting for the server goroutine to finish, but listener.Close() failing is extremely rare in practice.

No files require special attention; the single changed file is transport/tls/stream_dialer_test.go.

Important Files Changed

Filename Overview
transport/tls/stream_dialer_test.go Replaces badssl.com-backed TLS tests with local cert generation and a local TLS server; one minor cleanup ordering concern in serveTLSHandshakes.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Test
    participant serveTLSHandshakes
    participant TLSListener as TLS Listener (127.0.0.1:0)
    participant StreamDialer
    participant certVerifier as StandardCertVerifier

    Test->>serveTLSHandshakes: createRootCA / createLeafCert
    serveTLSHandshakes->>TLSListener: stdTLS.Listen("tcp", "127.0.0.1:0", ...)
    serveTLSHandshakes-->>Test: addr (127.0.0.1:PORT)

    Test->>StreamDialer: NewStreamDialer(TCPDialer, WithCertVerifier(verifier))
    Test->>StreamDialer: DialStream(ctx, addr)
    StreamDialer->>TLSListener: TCP connect
    TLSListener-->>StreamDialer: accepted conn
    par Client handshake
        StreamDialer->>TLSListener: TLS ClientHello
        TLSListener->>StreamDialer: TLS ServerHello + Certificate
        StreamDialer->>certVerifier: VerifyCertificate(PeerCertificates)
        certVerifier-->>StreamDialer: error (UnknownAuthority / Expired)
        StreamDialer->>TLSListener: TLS Alert
    and Server handshake goroutine
        TLSListener->>TLSListener: "Handshake() -> error (ignored with _)"
    end
    StreamDialer-->>Test: error (x509.UnknownAuthorityError or x509.CertificateInvalidError)
    Test->>Test: require.ErrorAs / require.Equal

    Note over Test,TLSListener: t.Cleanup: listener.Close() -> goroutine exits -> done
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Test
    participant serveTLSHandshakes
    participant TLSListener as TLS Listener (127.0.0.1:0)
    participant StreamDialer
    participant certVerifier as StandardCertVerifier

    Test->>serveTLSHandshakes: createRootCA / createLeafCert
    serveTLSHandshakes->>TLSListener: stdTLS.Listen("tcp", "127.0.0.1:0", ...)
    serveTLSHandshakes-->>Test: addr (127.0.0.1:PORT)

    Test->>StreamDialer: NewStreamDialer(TCPDialer, WithCertVerifier(verifier))
    Test->>StreamDialer: DialStream(ctx, addr)
    StreamDialer->>TLSListener: TCP connect
    TLSListener-->>StreamDialer: accepted conn
    par Client handshake
        StreamDialer->>TLSListener: TLS ClientHello
        TLSListener->>StreamDialer: TLS ServerHello + Certificate
        StreamDialer->>certVerifier: VerifyCertificate(PeerCertificates)
        certVerifier-->>StreamDialer: error (UnknownAuthority / Expired)
        StreamDialer->>TLSListener: TLS Alert
    and Server handshake goroutine
        TLSListener->>TLSListener: "Handshake() -> error (ignored with _)"
    end
    StreamDialer-->>Test: error (x509.UnknownAuthorityError or x509.CertificateInvalidError)
    Test->>Test: require.ErrorAs / require.Equal

    Note over Test,TLSListener: t.Cleanup: listener.Close() -> goroutine exits -> done
Loading

Reviews (1): Last reviewed commit: "fix tls cert tests to use local certific..." | Re-trigger Greptile

Comment thread transport/tls/stream_dialer_test.go Outdated
@fortuna fortuna disabled auto-merge June 16, 2026 21:10
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@fortuna fortuna enabled auto-merge (squash) June 16, 2026 21:11
@fortuna fortuna merged commit 15d0845 into main Jun 16, 2026
7 checks passed
@fortuna fortuna deleted the codex/tls-expired-cert-local branch June 16, 2026 22:00
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