Skip to content

[Repo Assist] refactor(httputil): centralize TLS min-version policy#7834

Merged
lpcox merged 2 commits into
mainfrom
repo-assist/improve-tls-config-central-7819-86c0db753b2a5bdb
Jun 20, 2026
Merged

[Repo Assist] refactor(httputil): centralize TLS min-version policy#7834
lpcox merged 2 commits into
mainfrom
repo-assist/improve-tls-config-central-7819-86c0db753b2a5bdb

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

🤖 This PR is from Repo Assist, an automated AI assistant for this repository.

Closes #7819

What

Addresses the TLS Config duplication identified in #7819 (part of duplicate code analysis #7818).

Three call sites previously hard-coded tls.VersionTLS12 independently:

File Before
internal/server/gateway_tls.go:36 &tls.Config{Certificates: []tls.Certificate{serverCert}, MinVersion: tls.VersionTLS12}
internal/proxy/tls.go:163 &tls.Config{Certificates: []tls.Certificate{serverCertPair}, MinVersion: tls.VersionTLS12}
internal/proxy/proxy.go:126 &tls.Config{MinVersion: tls.VersionTLS12}

This PR adds internal/httputil/tls.go with:

  • MinTLSVersion — exported constant (= tls.VersionTLS12) as the single source of truth for the gateway's minimum TLS version policy
  • NewServerTLSConfig(cert tls.Certificate) *tls.Config — helper for TLS server listeners
  • NewClientTLSConfig() *tls.Config — helper for outbound HTTP clients

Both server and proxy packages already import httputil, so no new dependency is introduced.

Why

A future TLS policy upgrade (e.g., to TLS 1.3) currently requires three coordinated changes across two packages. With this PR it becomes a single-line edit to httputil.MinTLSVersion. It also makes the intent explicit — a reader who sees httputil.NewServerTLSConfig(cert) immediately understands they're getting the standard gateway server TLS config.

Test Status

⚠️ Infrastructure note: proxy.golang.org is blocked by the environment firewall, preventing go build ./... and go test ./.... This is a known environment constraint affecting all recent Repo Assist runs.

Manual verification performed:

  • gofmt -e passes for all 4 changed files — syntax is valid
  • All three call sites produce the same *tls.Config as before (zero behaviour change)
  • gateway_tls.go retains its crypto/tls import (still uses tls.LoadX509KeyPair, tls.RequireAndVerifyClientCert, tls.VersionName)
  • proxy/tls.go retains its crypto/tls import (still uses tls.LoadX509KeyPair, tls.Config in struct type)
  • proxy/proxy.go no longer imports crypto/tls (sole usage was the now-replaced tls.Config{MinVersion: ...} literal)

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • proxy.golang.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "proxy.golang.org"

See Network Configuration for more information.

Generated by Repo Assist · 1.4K AIC · ⊞ 45.3K ·
Comment /repo-assist to run again

Add this agentic workflows to your repo

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@851905c06e905bf362a9f6cc54f912e3df747d55

Add NewServerTLSConfig and NewClientTLSConfig helpers to internal/httputil
along with a MinTLSVersion constant. Three call sites were previously
open-coding `tls.VersionTLS12` independently:

  internal/server/gateway_tls.go
  internal/proxy/tls.go
  internal/proxy/proxy.go

With this change, the minimum TLS version policy lives in one place
(httputil.MinTLSVersion). A future upgrade from TLS 1.2 to TLS 1.3 now
requires a single-line edit instead of three coordinated hunts.

Closes #7819

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox lpcox marked this pull request as ready for review June 20, 2026 16:34
Copilot AI review requested due to automatic review settings June 20, 2026 16:34

Copilot AI 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.

Pull request overview

This PR refactors TLS configuration setup to centralize the gateway’s minimum TLS version policy in internal/httputil, removing duplicated tls.Config{MinVersion: tls.VersionTLS12} literals across server and proxy code paths.

Changes:

  • Added internal/httputil/tls.go providing MinTLSVersion, NewServerTLSConfig, and NewClientTLSConfig helpers.
  • Updated gateway server TLS config creation to use httputil.NewServerTLSConfig.
  • Updated proxy TLS config creation (self-signed server TLS + outbound client TLS) to use the new httputil helpers.
Show a summary per file
File Description
internal/server/gateway_tls.go Replaces inline server tls.Config creation with shared httputil.NewServerTLSConfig.
internal/proxy/tls.go Replaces inline self-signed server tls.Config creation with shared httputil.NewServerTLSConfig.
internal/proxy/proxy.go Uses httputil.NewClientTLSConfig for HTTP client TLS policy; removes direct crypto/tls dependency in this file.
internal/httputil/tls.go Introduces centralized minimum TLS version constant and helper constructors for server/client TLS configs.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 4/4 changed files
  • Comments generated: 2

Comment thread internal/httputil/tls.go Outdated
Comment thread internal/httputil/tls.go
Comment on lines +12 to +18
// NewServerTLSConfig returns a *tls.Config for TLS server listeners carrying
// the provided certificate and the gateway-wide minimum TLS version.
func NewServerTLSConfig(cert tls.Certificate) *tls.Config {
return &tls.Config{
Certificates: []tls.Certificate{cert},
MinVersion: MinTLSVersion,
}
@lpcox

lpcox commented Jun 20, 2026

Copy link
Copy Markdown
Collaborator

@copilot address review feedback

Copilot AI commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

@copilot address review feedback

Addressed in 9eb53d7.

  • Updated internal/httputil/tls.go comment spelling (Centralizing)
  • Added internal/httputil/tls_test.go with coverage for NewServerTLSConfig and NewClientTLSConfig
  • Verified with go test ./internal/httputil and make agent-finished

Copilot finished work on behalf of lpcox June 20, 2026 16:42
Copilot AI requested a review from lpcox June 20, 2026 16:42
@lpcox lpcox merged commit a4cfdb6 into main Jun 20, 2026
26 checks passed
@lpcox lpcox deleted the repo-assist/improve-tls-config-central-7819-86c0db753b2a5bdb branch June 20, 2026 16:58
lpcox added a commit that referenced this pull request Jun 21, 2026
🤖 *This PR is from Repo Assist, an automated AI assistant for this
repository.*

Closes #7862

## Root Cause

After PR #7834 was merged (which introduced `internal/httputil/tls.go`),
a second file `internal/httputil/tls_config.go` was added to the same
package with identical declarations for `MinTLSVersion`,
`NewServerTLSConfig`, and `NewClientTLSConfig`. Go does not permit two
files in the same package to define the same exported identifiers — this
causes a compile error:

```
internal/httputil/tls_config.go:7:7: MinTLSVersion redeclared in this block
internal/httputil/tls_config.go:11:6: NewServerTLSConfig redeclared in this block
internal/httputil/tls_config.go:20:6: NewClientTLSConfig redeclared in this block
```

## Fix

- **Delete `internal/httputil/tls_config.go`** — `tls.go` is kept as it
has more detailed godoc
- **Delete `internal/httputil/tls_config_test.go`** — `tls_test.go` is
kept
- **Absorb `assert.NotNil` checks** from `tls_config_test.go` into
`tls_test.go` so no test coverage is lost

No production logic changes — only the duplicate file is removed.

## Test Status

⚠️ **Infrastructure note**: `proxy.golang.org` is blocked by the
environment firewall, preventing `go build ./...` and `go test ./...`.

**Manual verification performed:**
- `go tool compile -e internal/httputil/tls.go` — after removing
`tls_config.go`, no redeclaration errors remain (only the expected
"could not import crypto/tls" from the isolated invocation context)
- `gofmt -d internal/httputil/tls.go internal/httputil/tls_test.go` —
clean, no diffs
- Both files are syntactically valid Go




> [!WARNING]
> <details>
> <summary>Firewall blocked 1 domain</summary>
>
> The following domain was blocked by the firewall during workflow
execution:
>
> - `proxy.golang.org`
>> To allow these domains, add them to the `network.allowed` list in
your workflow frontmatter:
>
> ```yaml
> network:
>   allowed:
>     - defaults
>     - "proxy.golang.org"
> ```
>
> See [Network
Configuration](https://github.github.com/gh-aw/reference/network/) for
more information.
>
> </details>


> Generated by [Repo
Assist](https://github.com/github/gh-aw-mcpg/actions/runs/27905590368) ·
1.2K AIC · ⊞ 45.3K ·
[◷](https://github.com/search?q=repo%3Agithub%2Fgh-aw-mcpg+%22gh-aw-workflow-id%3A+repo-assist%22&type=pullrequests)
> <sub>Comment <em>/repo-assist</em> to run again</sub>
>
<details>
<summary>Add this agentic workflows to your repo</summary>

To install this agentic workflow, run

```
gh aw add githubnext/agentics@851905c
```
</details>


<!-- gh-aw-agentic-workflow: Repo Assist, engine: copilot, version:
1.0.60, model: claude-sonnet-4.6, id: 27905590368, workflow_id:
repo-assist, run:
https://github.com/github/gh-aw-mcpg/actions/runs/27905590368 -->

<!-- gh-aw-workflow-id: repo-assist -->
<!-- gh-aw-workflow-call-id: github/gh-aw-mcpg/repo-assist -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[duplicate-code] Duplicate Code Pattern: TLS Config Struct Initialization (3 locations)

3 participants