From 2952c3d3bce8e7fc6183ce500cc4144fed33c2b8 Mon Sep 17 00:00:00 2001 From: Emre Tinaztepe Date: Fri, 6 Mar 2026 12:20:28 +0000 Subject: [PATCH] fix: Address PR review feedback - Reuse cli.Config.LoadRootCAs() instead of reimplementing CA loading - Return errors from resolveConfig instead of log.Fatal for testability - Add goreleaser entries for wardgate-proxy (build, archive, brew) - Add unit tests for resolveConfig error paths Co-Authored-By: Claude Opus 4.6 -e Signed-off-by: Emre Tinaztepe --- .goreleaser.yaml | 42 +++++++++++++ cmd/wardgate-proxy/main.go | 41 +++++-------- cmd/wardgate-proxy/main_test.go | 101 ++++++++++++++++++++++++++++++++ 3 files changed, 159 insertions(+), 25 deletions(-) diff --git a/.goreleaser.yaml b/.goreleaser.yaml index 8346301..776c31b 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -56,6 +56,24 @@ builds: - -X main.commit={{.Commit}} - -X main.date={{.Date}} + - id: wardgate-proxy + main: ./cmd/wardgate-proxy + binary: wardgate-proxy + env: + - CGO_ENABLED=0 + goos: + - linux + - darwin + - windows + goarch: + - amd64 + - arm64 + ldflags: + - -s -w + - -X main.version={{.Version}} + - -X main.commit={{.Commit}} + - -X main.date={{.Date}} + archives: - id: wardgate ids: @@ -95,6 +113,18 @@ archives: - README.md - LICENSE + - id: wardgate-proxy + ids: + - wardgate-proxy + formats: [tar.gz] + name_template: "wardgate-proxy_{{ .Version }}_{{ .Os }}_{{ .Arch }}" + format_overrides: + - goos: windows + formats: [zip] + files: + - README.md + - LICENSE + checksum: name_template: "checksums.txt" @@ -159,6 +189,18 @@ brews: install: | bin.install "wardgate-exec" + - name: wardgate-proxy + ids: [wardgate-proxy] + repository: + owner: wardgate + name: homebrew-wardgate + token: "{{ .Env.HOMEBREW_TAP_GITHUB_TOKEN }}" + homepage: "https://github.com/wardgate/wardgate" + description: "API gateway for AI agents - local reverse proxy" + license: "AGPL-3.0" + install: | + bin.install "wardgate-proxy" + release: github: owner: wardgate diff --git a/cmd/wardgate-proxy/main.go b/cmd/wardgate-proxy/main.go index d8a4d3f..4fed649 100644 --- a/cmd/wardgate-proxy/main.go +++ b/cmd/wardgate-proxy/main.go @@ -3,7 +3,6 @@ package main import ( "context" "crypto/tls" - "crypto/x509" "flag" "fmt" "log" @@ -17,6 +16,7 @@ import ( "syscall" "time" + "github.com/wardgate/wardgate/internal/cli" "gopkg.in/yaml.v3" ) @@ -132,24 +132,12 @@ func (cr *configKeyReader) Read() (string, error) { } func buildTransport(caFile string) (*http.Transport, error) { - tlsCfg := &tls.Config{} - - if caFile != "" { - pem, err := os.ReadFile(caFile) - if err != nil { - return nil, fmt.Errorf("read ca_file: %w", err) - } - pool, err := x509.SystemCertPool() - if err != nil { - pool = x509.NewCertPool() - } - if !pool.AppendCertsFromPEM(pem) { - return nil, fmt.Errorf("ca_file: no valid PEM certificates") - } - tlsCfg.RootCAs = pool + cliCfg := &cli.Config{CAFile: caFile} + rootCAs, err := cliCfg.LoadRootCAs() + if err != nil { + return nil, err } - - return &http.Transport{TLSClientConfig: tlsCfg}, nil + return &http.Transport{TLSClientConfig: &tls.Config{RootCAs: rootCAs}}, nil } // NewProxyHandler builds an http.Handler that reads an agent key from keyReader @@ -188,16 +176,16 @@ type contextKey string const ctxAgentKey contextKey = "agentKey" // resolveConfig loads the config file and applies flag overrides. -func resolveConfig(configPath string, configExplicit bool, listen, server, keyEnv string) *Config { +func resolveConfig(configPath string, configExplicit bool, listen, server, keyEnv string) (*Config, error) { cfg := &Config{Listen: "127.0.0.1:18080"} data, err := os.ReadFile(configPath) if err != nil { if configExplicit { - log.Fatalf("Error: config file %s not found", configPath) + return nil, fmt.Errorf("config file %s not found", configPath) } } else { if err := yaml.Unmarshal(data, cfg); err != nil { - log.Fatalf("Error parsing config %s: %v", configPath, err) + return nil, fmt.Errorf("parsing config %s: %w", configPath, err) } } @@ -215,12 +203,12 @@ func resolveConfig(configPath string, configExplicit bool, listen, server, keyEn cfg.Listen = "127.0.0.1:18080" } if cfg.Server == "" { - log.Fatal("Error: server URL not configured (set server in config or use -server flag)") + return nil, fmt.Errorf("server URL not configured (set server in config or use -server flag)") } if cfg.Key == "" && cfg.KeyEnv == "" { - log.Fatal("Error: agent key not configured (set key or key_env in config, or use -key-env flag)") + return nil, fmt.Errorf("agent key not configured (set key or key_env in config, or use -key-env flag)") } - return cfg + return cfg, nil } func main() { @@ -243,7 +231,10 @@ func main() { } }) - cfg := resolveConfig(*configPath, configExplicit, *listenFlag, *serverFlag, *keyEnvFlag) + cfg, err := resolveConfig(*configPath, configExplicit, *listenFlag, *serverFlag, *keyEnvFlag) + if err != nil { + log.Fatalf("Error: %v", err) + } upstream, err := url.Parse(cfg.Server) if err != nil { diff --git a/cmd/wardgate-proxy/main_test.go b/cmd/wardgate-proxy/main_test.go index f5e1dcf..73986c1 100644 --- a/cmd/wardgate-proxy/main_test.go +++ b/cmd/wardgate-proxy/main_test.go @@ -295,6 +295,107 @@ func TestLoadConfig_InvalidYAML(t *testing.T) { } } +// --- resolveConfig tests --- + +func TestResolveConfig_ExplicitMissingFile(t *testing.T) { + _, err := resolveConfig("/nonexistent/config.yaml", true, "", "", "") + if err == nil { + t.Fatal("expected error for explicit missing config") + } + if !strings.Contains(err.Error(), "not found") { + t.Errorf("expected 'not found' in error, got: %v", err) + } +} + +func TestResolveConfig_InvalidYAML(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + os.WriteFile(path, []byte("{{{invalid"), 0o600) + + _, err := resolveConfig(path, true, "", "", "") + if err == nil { + t.Fatal("expected error for invalid YAML") + } + if !strings.Contains(err.Error(), "parsing config") { + t.Errorf("expected 'parsing config' in error, got: %v", err) + } +} + +func TestResolveConfig_MissingServer(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + os.WriteFile(path, []byte("key: my-key\n"), 0o600) + + _, err := resolveConfig(path, true, "", "", "") + if err == nil { + t.Fatal("expected error for missing server") + } + if !strings.Contains(err.Error(), "server URL not configured") { + t.Errorf("expected 'server URL not configured' in error, got: %v", err) + } +} + +func TestResolveConfig_MissingKey(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + os.WriteFile(path, []byte("server: https://example.com\n"), 0o600) + + _, err := resolveConfig(path, true, "", "", "") + if err == nil { + t.Fatal("expected error for missing key") + } + if !strings.Contains(err.Error(), "agent key not configured") { + t.Errorf("expected 'agent key not configured' in error, got: %v", err) + } +} + +func TestResolveConfig_ValidConfig(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + os.WriteFile(path, []byte("server: https://example.com\nkey: my-key\nlisten: 127.0.0.1:9090\n"), 0o600) + + cfg, err := resolveConfig(path, true, "", "", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.Server != "https://example.com" { + t.Errorf("server: got %q", cfg.Server) + } + if cfg.Key != "my-key" { + t.Errorf("key: got %q", cfg.Key) + } + if cfg.Listen != "127.0.0.1:9090" { + t.Errorf("listen: got %q", cfg.Listen) + } +} + +func TestResolveConfig_FlagOverrides(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + os.WriteFile(path, []byte("server: https://example.com\nkey: my-key\n"), 0o600) + + cfg, err := resolveConfig(path, true, "0.0.0.0:8080", "https://override.com", "MY_KEY_ENV") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.Listen != "0.0.0.0:8080" { + t.Errorf("listen: got %q, want 0.0.0.0:8080", cfg.Listen) + } + if cfg.Server != "https://override.com" { + t.Errorf("server: got %q, want https://override.com", cfg.Server) + } + if cfg.KeyEnv != "MY_KEY_ENV" { + t.Errorf("key_env: got %q, want MY_KEY_ENV", cfg.KeyEnv) + } +} + +func TestResolveConfig_ImplicitMissingFileUsesDefaults(t *testing.T) { + _, err := resolveConfig("/nonexistent/config.yaml", false, "", "https://example.com", "MY_KEY") + if err != nil { + t.Fatalf("unexpected error for implicit missing config with flag overrides: %v", err) + } +} + // --- Proxy handler unit tests --- func TestProxyHandler_InjectsBearer(t *testing.T) {