From 8b53642bd0cc21c42449fe381c07ea914b2d9585 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Jul 2026 20:31:59 +0000 Subject: [PATCH 1/2] Initial plan From a3727313b73c401b787ee7aa2a1a087c97d7cc61 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Jul 2026 20:41:28 +0000 Subject: [PATCH 2/2] =?UTF-8?q?refactor:=20consolidate=20TLS=20helpers=20?= =?UTF-8?q?=E2=80=94=20move=20LoadGatewayTLS=20from=20server=20to=20httput?= =?UTF-8?q?il?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- internal/cmd/root.go | 3 +- internal/httputil/tls.go | 56 ++++++++++++++++- .../tls_gateway_test.go} | 15 ++--- internal/logger/logger_namespace_test.go | 1 - internal/proxy/tls.go | 6 +- internal/server/gateway_tls.go | 61 ------------------- 6 files changed, 66 insertions(+), 76 deletions(-) rename internal/{server/gateway_tls_test.go => httputil/tls_gateway_test.go} (93%) delete mode 100644 internal/server/gateway_tls.go diff --git a/internal/cmd/root.go b/internal/cmd/root.go index e44b1f339..2664e33bf 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -16,6 +16,7 @@ import ( "github.com/github/gh-aw-mcpg/internal/difc" "github.com/github/gh-aw-mcpg/internal/envutil" "github.com/github/gh-aw-mcpg/internal/guard" + "github.com/github/gh-aw-mcpg/internal/httputil" "github.com/github/gh-aw-mcpg/internal/logger" "github.com/github/gh-aw-mcpg/internal/server" "github.com/github/gh-aw-mcpg/internal/tracing" @@ -450,7 +451,7 @@ func run(cmd *cobra.Command, args []string) error { tlsEnabled := hasCert && hasKey var tlsCfg *tls.Config if tlsEnabled { - tlsCfg, err = server.LoadGatewayTLS(tlsCertPath, tlsKeyPath, tlsCAPath) + tlsCfg, err = httputil.LoadGatewayTLS(tlsCertPath, tlsKeyPath, tlsCAPath) if err != nil { _ = listener.Close() return fmt.Errorf("failed to configure TLS: %w", err) diff --git a/internal/httputil/tls.go b/internal/httputil/tls.go index 163f57e8f..fc908e1b9 100644 --- a/internal/httputil/tls.go +++ b/internal/httputil/tls.go @@ -2,9 +2,10 @@ // // TLS helpers are split across two packages: // -// - internal/httputil (this package): protocol-level helpers that apply to -// all TLS listeners and clients (MinTLSVersion, NewServerTLSConfig, -// NewClientTLSConfig, ConfigureTLSTrustEnvironment). +// - internal/httputil (this file): protocol-level helpers and file-loading +// helpers that apply to all TLS listeners and clients: +// MinTLSVersion, NewServerTLSConfig, NewClientTLSConfig, +// ConfigureTLSTrustEnvironment, LoadGatewayTLS. // // - internal/proxy: certificate *generation* (GenerateSelfSignedTLS) lives // there because it is only needed when the proxy runs in self-signed mode. @@ -13,6 +14,7 @@ package httputil import ( "crypto/tls" + "crypto/x509" "fmt" "os" "strings" @@ -80,3 +82,51 @@ func ConfigureTLSTrustEnvironment(caCertPath string) error { logTLS.Printf("TLS trust environment configured successfully: %d env vars set", len(tlsTrustEnvKeys)) return nil } + +// LoadGatewayTLS loads a TLS configuration for the gateway HTTP server from PEM +// certificate and key files. When caPath is non-empty the returned config +// requires client certificates signed by that CA (mutual TLS / mTLS). +// +// Pass an empty caPath to use one-way TLS (server-only authentication). +// +// Example — one-way TLS (server cert only): +// +// tlsCfg, err := LoadGatewayTLS("/path/server.crt", "/path/server.key", "") +// +// Example — mutual TLS (client certs required): +// +// tlsCfg, err := LoadGatewayTLS("/path/server.crt", "/path/server.key", "/path/ca.crt") +func LoadGatewayTLS(certPath, keyPath, caPath string) (*tls.Config, error) { + logTLS.Printf("loading gateway TLS: cert=%s, key=%s, ca=%s", certPath, keyPath, caPath) + + serverCert, err := tls.LoadX509KeyPair(certPath, keyPath) + if err != nil { + return nil, fmt.Errorf("failed to load server TLS certificate/key: %w", err) + } + logTLS.Printf("server TLS key pair loaded: certChainLen=%d", len(serverCert.Certificate)) + + cfg := NewServerTLSConfig(serverCert) + + if caPath != "" { + caPEM, err := os.ReadFile(caPath) + if err != nil { + return nil, fmt.Errorf("failed to read CA certificate: %w", err) + } + + caPool := x509.NewCertPool() + if !caPool.AppendCertsFromPEM(caPEM) { + return nil, fmt.Errorf("failed to parse CA certificate from %s", caPath) + } + logTLS.Printf("CA certificate pool built: ca=%s", caPath) + + // Require and verify client certificates signed by the provided CA. + cfg.ClientCAs = caPool + cfg.ClientAuth = tls.RequireAndVerifyClientCert + logTLS.Printf("mTLS enabled: client certificates required, CA=%s", caPath) + } else { + logTLS.Print("one-way TLS configured: client certificates not required") + } + + logTLS.Printf("gateway TLS configuration ready: minVersion=%s, mtls=%v", tls.VersionName(cfg.MinVersion), caPath != "") + return cfg, nil +} diff --git a/internal/server/gateway_tls_test.go b/internal/httputil/tls_gateway_test.go similarity index 93% rename from internal/server/gateway_tls_test.go rename to internal/httputil/tls_gateway_test.go index 3135e1060..34d37ca64 100644 --- a/internal/server/gateway_tls_test.go +++ b/internal/httputil/tls_gateway_test.go @@ -1,4 +1,4 @@ -package server +package httputil_test import ( "crypto/ecdsa" @@ -20,6 +20,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/github/gh-aw-mcpg/internal/httputil" "github.com/github/gh-aw-mcpg/internal/proxy" ) @@ -214,7 +215,7 @@ func TestLoadGatewayTLS_ServerOnly(t *testing.T) { tlsCfg, err := proxy.GenerateSelfSignedTLS(dir) require.NoError(t, err) - cfg, err := LoadGatewayTLS(tlsCfg.CertPath, tlsCfg.KeyPath, "") + cfg, err := httputil.LoadGatewayTLS(tlsCfg.CertPath, tlsCfg.KeyPath, "") require.NoError(t, err) require.NotNil(t, cfg) @@ -229,7 +230,7 @@ func TestLoadGatewayTLS_MutualTLS(t *testing.T) { tlsCfg, err := proxy.GenerateSelfSignedTLS(dir) require.NoError(t, err) - cfg, err := LoadGatewayTLS(tlsCfg.CertPath, tlsCfg.KeyPath, tlsCfg.CACertPath) + cfg, err := httputil.LoadGatewayTLS(tlsCfg.CertPath, tlsCfg.KeyPath, tlsCfg.CACertPath) require.NoError(t, err) require.NotNil(t, cfg) @@ -242,7 +243,7 @@ func TestLoadGatewayTLS_ServerServesMTLS(t *testing.T) { certs, err := generateMTLSCerts(t, dir) require.NoError(t, err) - cfg, err := LoadGatewayTLS(certs.serverCertPath, certs.serverKeyPath, certs.caCertPath) + cfg, err := httputil.LoadGatewayTLS(certs.serverCertPath, certs.serverKeyPath, certs.caCertPath) require.NoError(t, err) srv := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { @@ -267,7 +268,7 @@ func TestLoadGatewayTLS_ServerServesMTLS(t *testing.T) { } func TestLoadGatewayTLS_InvalidCertPath(t *testing.T) { - _, err := LoadGatewayTLS("/nonexistent/cert.pem", "/nonexistent/key.pem", "") + _, err := httputil.LoadGatewayTLS("/nonexistent/cert.pem", "/nonexistent/key.pem", "") require.Error(t, err) assert.ErrorContains(t, err, "failed to load server TLS certificate/key") } @@ -277,7 +278,7 @@ func TestLoadGatewayTLS_InvalidCAPath(t *testing.T) { tlsCfg, err := proxy.GenerateSelfSignedTLS(dir) require.NoError(t, err) - _, err = LoadGatewayTLS(tlsCfg.CertPath, tlsCfg.KeyPath, "/nonexistent/ca.pem") + _, err = httputil.LoadGatewayTLS(tlsCfg.CertPath, tlsCfg.KeyPath, "/nonexistent/ca.pem") require.Error(t, err) assert.ErrorContains(t, err, "failed to read CA certificate") } @@ -291,7 +292,7 @@ func TestLoadGatewayTLS_MalformedCA(t *testing.T) { badCA := dir + "/bad-ca.pem" require.NoError(t, os.WriteFile(badCA, []byte("NOT A VALID PEM"), 0644)) - _, err = LoadGatewayTLS(tlsCfg.CertPath, tlsCfg.KeyPath, badCA) + _, err = httputil.LoadGatewayTLS(tlsCfg.CertPath, tlsCfg.KeyPath, badCA) require.Error(t, err) assert.ErrorContains(t, err, "failed to parse CA certificate") } diff --git a/internal/logger/logger_namespace_test.go b/internal/logger/logger_namespace_test.go index cbfcd9107..ec114e0cd 100644 --- a/internal/logger/logger_namespace_test.go +++ b/internal/logger/logger_namespace_test.go @@ -33,7 +33,6 @@ func TestLoggerNamespacesMatchFileConventions(t *testing.T) { "internal/guard/write_sink.go": {"guard:write-sink"}, "internal/launcher/connection_pool.go": {"launcher:pool"}, "internal/launcher/health_monitor.go": {"launcher:health"}, - "internal/server/gateway_tls.go": {"server:tls"}, "internal/server/http_helpers.go": {"server:helpers"}, "internal/server/http_server.go": {"server:http_server", "server:transport"}, "internal/server/middleware_auth.go": {"server:auth"}, diff --git a/internal/proxy/tls.go b/internal/proxy/tls.go index 2a90d7ac6..97407f562 100644 --- a/internal/proxy/tls.go +++ b/internal/proxy/tls.go @@ -14,9 +14,9 @@ // - internal/proxy (this file): certificate *generation* (GenerateSelfSignedTLS). // This is the only place self-signed certs are created. // -// - internal/httputil: protocol-level helpers that apply to all TLS listeners -// and clients (MinTLSVersion, NewServerTLSConfig, NewClientTLSConfig, -// ConfigureTLSTrustEnvironment). +// - internal/httputil: protocol-level helpers and file-loading helpers that +// apply to all TLS listeners and clients (MinTLSVersion, NewServerTLSConfig, +// NewClientTLSConfig, ConfigureTLSTrustEnvironment, LoadGatewayTLS). package proxy import ( diff --git a/internal/server/gateway_tls.go b/internal/server/gateway_tls.go deleted file mode 100644 index 42121b434..000000000 --- a/internal/server/gateway_tls.go +++ /dev/null @@ -1,61 +0,0 @@ -package server - -import ( - "crypto/tls" - "crypto/x509" - "fmt" - "os" - - "github.com/github/gh-aw-mcpg/internal/httputil" - "github.com/github/gh-aw-mcpg/internal/logger" -) - -var logGatewayTLS = logger.New("server:tls") - -// LoadGatewayTLS loads a TLS configuration for the gateway HTTP server from PEM -// certificate and key files. When caPath is non-empty the returned config -// requires client certificates signed by that CA (mutual TLS / mTLS). -// -// Pass an empty caPath to use one-way TLS (server-only authentication). -// -// Example — one-way TLS (server cert only): -// -// tlsCfg, err := LoadGatewayTLS("/path/server.crt", "/path/server.key", "") -// -// Example — mutual TLS (client certs required): -// -// tlsCfg, err := LoadGatewayTLS("/path/server.crt", "/path/server.key", "/path/ca.crt") -func LoadGatewayTLS(certPath, keyPath, caPath string) (*tls.Config, error) { - logGatewayTLS.Printf("loading gateway TLS: cert=%s, key=%s, ca=%s", certPath, keyPath, caPath) - - serverCert, err := tls.LoadX509KeyPair(certPath, keyPath) - if err != nil { - return nil, fmt.Errorf("failed to load server TLS certificate/key: %w", err) - } - logGatewayTLS.Printf("server TLS key pair loaded: certChainLen=%d", len(serverCert.Certificate)) - - cfg := httputil.NewServerTLSConfig(serverCert) - - if caPath != "" { - caPEM, err := os.ReadFile(caPath) - if err != nil { - return nil, fmt.Errorf("failed to read CA certificate: %w", err) - } - - caPool := x509.NewCertPool() - if !caPool.AppendCertsFromPEM(caPEM) { - return nil, fmt.Errorf("failed to parse CA certificate from %s", caPath) - } - logGatewayTLS.Printf("CA certificate pool built: ca=%s", caPath) - - // Require and verify client certificates signed by the provided CA. - cfg.ClientCAs = caPool - cfg.ClientAuth = tls.RequireAndVerifyClientCert - logGatewayTLS.Printf("mTLS enabled: client certificates required, CA=%s", caPath) - } else { - logGatewayTLS.Print("one-way TLS configured: client certificates not required") - } - - logGatewayTLS.Printf("gateway TLS configuration ready: minVersion=%s, mtls=%v", tls.VersionName(cfg.MinVersion), caPath != "") - return cfg, nil -}