From 923ef0122ed3ae66d3979176bdf897ac7c67a975 Mon Sep 17 00:00:00 2001 From: Natalie Gaston Date: Fri, 7 Aug 2026 16:42:26 -0700 Subject: [PATCH 1/4] refactor: add validation to vault address --- config/config.go | 101 +++++++++++++++ config/config_test.go | 286 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 387 insertions(+) diff --git a/config/config.go b/config/config.go index 74f238dfd..5b3605b7e 100644 --- a/config/config.go +++ b/config/config.go @@ -3,9 +3,12 @@ package config import ( "errors" "flag" + "fmt" "net" + "net/url" "os" "path/filepath" + "strings" "time" "github.com/ilyakaznacheev/cleanenv" @@ -17,6 +20,14 @@ var ConsoleConfig *Config // TrayMode indicates whether to run with system tray UI. var TrayMode bool +// Validation errors for configuration. +var ( + ErrSecretsAddrInsecure = errors.New("SECRETS_ADDR must use HTTPS for non-localhost addresses") + ErrSecretsAddrInvalid = errors.New("invalid SECRETS_ADDR") + ErrSecretsAddrMissingScheme = errors.New("SECRETS_ADDR missing scheme (use http:// or https://)") + ErrSecretsAddrNoHost = errors.New("SECRETS_ADDR contains no host") +) + const defaultHost = "localhost" type ( @@ -329,5 +340,95 @@ func NewConfig() (*Config, error) { return nil, err } + if err := ConsoleConfig.Validate(); err != nil { + return nil, err + } + return ConsoleConfig, nil } + +// Validate checks configuration for security and correctness issues. +func (c *Config) Validate() error { + // Ensure non-localhost Vault addresses use HTTPS + if c.Secrets.Address != "" { //nolint:staticcheck // QF1008: explicit field reference is clearer + if err := c.validateSecretsAddr(); err != nil { + return err + } + } + + return nil +} + +// validateSecretsAddr ensures Vault address uses HTTPS (except for localhost). +func (c *Config) validateSecretsAddr() error { + parsed, err := url.Parse(c.Secrets.Address) //nolint:staticcheck // QF1008: explicit field reference is clearer + if err != nil { + return fmt.Errorf("%w: %w", ErrSecretsAddrInvalid, err) + } + + // Check for valid scheme (must be http or https) + if parsed.Scheme != "http" && parsed.Scheme != "https" { + if !strings.Contains(c.Secrets.Address, "://") { //nolint:staticcheck // QF1008: explicit field reference is clearer + return fmt.Errorf("%w: %q", ErrSecretsAddrMissingScheme, c.Secrets.Address) //nolint:staticcheck // QF1008: explicit field reference is clearer + } + + return fmt.Errorf("%w: unsupported scheme %q in %q", ErrSecretsAddrInvalid, parsed.Scheme, c.Secrets.Address) //nolint:staticcheck // QF1008: explicit field reference is clearer + } + + // Check host is present before checking HTTPS requirement + hostname := parsed.Hostname() + if hostname == "" { + return fmt.Errorf("%w: %q", ErrSecretsAddrNoHost, c.Secrets.Address) //nolint:staticcheck // QF1008: explicit field reference is clearer + } + + // Enforce HTTPS for non-localhost + if parsed.Scheme == "http" && !isLocalhost(hostname) { + return ErrSecretsAddrInsecure + } + + return nil +} + +func isLocalhost(host string) bool { + hostOnly := stripPort(host) + + if strings.EqualFold(strings.TrimSuffix(hostOnly, "."), defaultHost) { + return true + } + + if ip := net.ParseIP(hostOnly); ip != nil { + return ip.IsLoopback() + } + + return false +} + +// stripPort removes port from host, handling both IPv4/hostname and IPv6 formats. +func stripPort(host string) string { + // Handle IPv6 addresses with brackets [::1]:port or just [::1] + if strings.HasPrefix(host, "[") { + if idx := strings.Index(host, "]"); idx != -1 { + // Check what comes after the closing bracket + afterBracket := host[idx+1:] + if afterBracket == "" || strings.HasPrefix(afterBracket, ":") { + // Valid format: [::1] or [::1]:port + return host[1:idx] + } + // Malformed format (e.g., [::1]incomplete) - return original + return host + } + + return host + } + + // IPv4, hostname, or unbracketed IPv6 - remove port carefully + // Unbracketed IPv6 addresses like ::1 have multiple colons + // Ports are always indicated by a single colon after the host + if strings.Count(host, ":") == 1 { + if idx := strings.LastIndex(host, ":"); idx != -1 { + return host[:idx] + } + } + + return host +} diff --git a/config/config_test.go b/config/config_test.go index 24e4360e7..4164883e1 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -1,6 +1,7 @@ package config import ( + "errors" "os" "testing" @@ -13,6 +14,19 @@ func clearEnv() { os.Unsetenv("LOG_LEVEL") os.Unsetenv("DB_POOL_MAX") os.Unsetenv("DB_URL") + os.Unsetenv("SECRETS_ADDR") +} + +func TestNewConfig_InvalidEnvVar(t *testing.T) { + clearEnv() + defer clearEnv() + + // DB_POOL_MAX expects an int; a non-numeric value causes cleanenv.ReadEnv to fail. + t.Setenv("DB_POOL_MAX", "not-a-number") + + cfg, err := NewConfig() + assert.Error(t, err) + assert.Nil(t, cfg) } func TestNewConfig_Defaults(t *testing.T) { //nolint:paralleltest // cannot have simultaneous tests modifying environment variables @@ -107,3 +121,275 @@ postgres: assert.Equal(t, 10, cfg.PoolMax) assert.Equal(t, "postgres://envuser:envpassword@localhost:5432/envdb", cfg.DB.URL) } + +func TestValidate_SecretsAddrEmpty(t *testing.T) { + t.Parallel() + + cfg := &Config{ + Secrets: Secrets{ + Address: "", + }, + } + assert.NoError(t, cfg.Validate()) +} + +func TestValidate_SecretsAddrHTTPSNonLocalhost(t *testing.T) { + t.Parallel() + + cfg := &Config{ + Secrets: Secrets{ + Address: "https://vault.example.com:8200", + }, + } + assert.NoError(t, cfg.Validate()) +} + +func TestValidate_SecretsAddrHTTPLocalhost(t *testing.T) { + t.Parallel() + + testCases := []string{ + "http://localhost:8200", + "http://127.0.0.1:8200", + "http://127.0.0.2:8200", + "http://[::1]:8200", + "http://[::1]", + } + for _, addr := range testCases { + t.Run(addr, func(t *testing.T) { + t.Parallel() + + cfg := &Config{ + Secrets: Secrets{ + Address: addr, + }, + } + assert.NoError(t, cfg.Validate(), "expected %s to be valid", addr) + }) + } +} + +func TestValidate_SecretsAddrHTTPNonLocalhost(t *testing.T) { + t.Parallel() + + testCases := []string{ + "http://vault.example.com:8200", + "http://192.168.1.1:8200", + "http://vault-server:8200", + } + for _, addr := range testCases { + t.Run(addr, func(t *testing.T) { + t.Parallel() + + cfg := &Config{ + Secrets: Secrets{ + Address: addr, + }, + } + err := cfg.Validate() + assert.Error(t, err, "expected %s to fail", addr) + assert.True(t, errors.Is(err, ErrSecretsAddrInsecure), "expected ErrSecretsAddrInsecure, got %v", err) + }) + } +} + +func TestValidate_SecretsAddrInvalidURL(t *testing.T) { + t.Parallel() + + cfg := &Config{ + Secrets: Secrets{ + Address: "://invalid", + }, + } + err := cfg.Validate() + assert.Error(t, err) + assert.True(t, errors.Is(err, ErrSecretsAddrInvalid), "expected ErrSecretsAddrInvalid, got %v", err) +} + +func TestValidate_SecretsAddrMissingScheme(t *testing.T) { + t.Parallel() + + testCases := []string{ + "vault.example.com:8200", + "localhost:8200", + } + + for _, addr := range testCases { + t.Run(addr, func(t *testing.T) { + t.Parallel() + + cfg := &Config{ + Secrets: Secrets{ + Address: addr, + }, + } + + err := cfg.Validate() + assert.Error(t, err) + assert.True(t, errors.Is(err, ErrSecretsAddrMissingScheme), "expected ErrSecretsAddrMissingScheme, got %v", err) + }) + } +} + +func TestValidate_SecretsAddrUnsupportedScheme(t *testing.T) { + t.Parallel() + + testCases := []string{ + "ftp://vault.example.com:8200", + "file:///tmp/vault", + } + + for _, addr := range testCases { + t.Run(addr, func(t *testing.T) { + t.Parallel() + + cfg := &Config{ + Secrets: Secrets{ + Address: addr, + }, + } + + err := cfg.Validate() + assert.Error(t, err) + assert.True(t, errors.Is(err, ErrSecretsAddrInvalid), "expected ErrSecretsAddrInvalid, got %v", err) + assert.Contains(t, err.Error(), "unsupported scheme") + }) + } +} + +func TestIsLocalhost(t *testing.T) { + t.Parallel() + + testCases := []struct { + host string + expected bool + }{ + // Localhost variants + {"localhost", true}, + {"LOCALHOST", true}, + {"localhost.", true}, + {"LOCALHOST.", true}, + {"localhost:8200", true}, + {"LOCALHOST.:8200", true}, + {"127.0.0.1", true}, + {"127.0.0.1:8200", true}, + {"127.255.255.255", true}, + {"127.1.1.1:9000", true}, + {"[::1]", true}, + {"::1", true}, + {"[::1]:8200", true}, + + // Non-localhost + {"192.168.1.1", false}, + {"vault.example.com", false}, + {"vault.example.com:8200", false}, + {"172.16.0.1", false}, + {"example.com", false}, + {"[2001:db8::1]", false}, + {"2001:db8::1", false}, + } + for _, tc := range testCases { + t.Run(tc.host, func(t *testing.T) { + t.Parallel() + + result := isLocalhost(tc.host) + assert.Equal(t, tc.expected, result, "isLocalhost(%s) = %v, want %v", tc.host, result, tc.expected) + }) + } +} + +func TestValidate_CallsValidateSecretsAddr(t *testing.T) { + t.Parallel() + + cfg := &Config{ + Secrets: Secrets{ + Address: "http://vault.example.com:8200", // Remote HTTP - should fail + }, + } + + err := cfg.Validate() + assert.Error(t, err) + assert.True(t, errors.Is(err, ErrSecretsAddrInsecure), "expected ErrSecretsAddrInsecure, got %v", err) +} + +func TestValidate_AllowsValidRemoteHTTPS(t *testing.T) { + t.Parallel() + + cfg := &Config{ + Secrets: Secrets{ + Address: "https://vault.example.com:8200", + }, + } + + err := cfg.Validate() + assert.NoError(t, err) +} + +func TestValidateSecretsAddr_NoHost(t *testing.T) { + t.Parallel() + + testCases := []string{ + "http://", // Scheme but no host + "https://", // Scheme but no host + "http://:8200", // Scheme with port but no host + "https://:8200", // Scheme with port but no host + } + for _, addr := range testCases { + t.Run(addr, func(t *testing.T) { + t.Parallel() + + cfg := &Config{ + Secrets: Secrets{ + Address: addr, + }, + } + err := cfg.validateSecretsAddr() + assert.Error(t, err, "expected %s to fail", addr) + assert.True(t, errors.Is(err, ErrSecretsAddrNoHost), "expected ErrSecretsAddrNoHost, got %v", err) + }) + } +} + +func TestStripPort(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + input string + expected string + }{ + // IPv6 with brackets + "[::1]:8200": {input: "[::1]:8200", expected: "::1"}, + "[::1]": {input: "[::1]", expected: "::1"}, + "[fe80::1]:9090": {input: "[fe80::1]:9090", expected: "fe80::1"}, + "[2001:db8::1]:443": {input: "[2001:db8::1]:443", expected: "2001:db8::1"}, + + // IPv4 with port + "192.168.1.1:8200": {input: "192.168.1.1:8200", expected: "192.168.1.1"}, + "127.0.0.1:9090": {input: "127.0.0.1:9090", expected: "127.0.0.1"}, + + // Hostname with port + "localhost:8200": {input: "localhost:8200", expected: "localhost"}, + "vault.example.com:8200": {input: "vault.example.com:8200", expected: "vault.example.com"}, + + // No port (returned as-is by final return statement on line 407) + "localhost": {input: "localhost", expected: "localhost"}, + "192.168.1.1": {input: "192.168.1.1", expected: "192.168.1.1"}, + "127.0.0.1": {input: "127.0.0.1", expected: "127.0.0.1"}, + "vault.example.com": {input: "vault.example.com", expected: "vault.example.com"}, + "::1": {input: "::1", expected: "::1"}, // Unbracketed IPv6, no port + "fe80::1": {input: "fe80::1", expected: "fe80::1"}, // Unbracketed IPv6 with multiple colons + "2001:db8::1": {input: "2001:db8::1", expected: "2001:db8::1"}, // Full IPv6, no port + + // Edge cases + "[::1]incomplete": {input: "[::1]incomplete", expected: "[::1]incomplete"}, // Malformed: extra suffix after closing bracket returned as-is + "localhost:": {input: "localhost:", expected: "localhost"}, // Trailing colon + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + t.Parallel() + + result := stripPort(tc.input) + assert.Equal(t, tc.expected, result, "stripPort(%q) = %q, want %q", tc.input, result, tc.expected) + }) + } +} From fc758a8ce7623f3d4e57e68524d7e9b4bfa86919 Mon Sep 17 00:00:00 2001 From: Natalie Gaston Date: Fri, 14 Aug 2026 13:36:49 -0700 Subject: [PATCH 2/4] refactor: remove dead code --- config/config.go | 36 +++-------------------------- config/config_test.go | 54 +------------------------------------------ 2 files changed, 4 insertions(+), 86 deletions(-) diff --git a/config/config.go b/config/config.go index 5b3605b7e..b59034a87 100644 --- a/config/config.go +++ b/config/config.go @@ -390,45 +390,15 @@ func (c *Config) validateSecretsAddr() error { } func isLocalhost(host string) bool { - hostOnly := stripPort(host) + host = strings.TrimSuffix(host, ".") - if strings.EqualFold(strings.TrimSuffix(hostOnly, "."), defaultHost) { + if strings.EqualFold(host, defaultHost) { return true } - if ip := net.ParseIP(hostOnly); ip != nil { + if ip := net.ParseIP(host); ip != nil { return ip.IsLoopback() } return false } - -// stripPort removes port from host, handling both IPv4/hostname and IPv6 formats. -func stripPort(host string) string { - // Handle IPv6 addresses with brackets [::1]:port or just [::1] - if strings.HasPrefix(host, "[") { - if idx := strings.Index(host, "]"); idx != -1 { - // Check what comes after the closing bracket - afterBracket := host[idx+1:] - if afterBracket == "" || strings.HasPrefix(afterBracket, ":") { - // Valid format: [::1] or [::1]:port - return host[1:idx] - } - // Malformed format (e.g., [::1]incomplete) - return original - return host - } - - return host - } - - // IPv4, hostname, or unbracketed IPv6 - remove port carefully - // Unbracketed IPv6 addresses like ::1 have multiple colons - // Ports are always indicated by a single colon after the host - if strings.Count(host, ":") == 1 { - if idx := strings.LastIndex(host, ":"); idx != -1 { - return host[:idx] - } - } - - return host -} diff --git a/config/config_test.go b/config/config_test.go index 4164883e1..b636abcf1 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -268,23 +268,16 @@ func TestIsLocalhost(t *testing.T) { {"LOCALHOST", true}, {"localhost.", true}, {"LOCALHOST.", true}, - {"localhost:8200", true}, - {"LOCALHOST.:8200", true}, {"127.0.0.1", true}, - {"127.0.0.1:8200", true}, {"127.255.255.255", true}, - {"127.1.1.1:9000", true}, - {"[::1]", true}, {"::1", true}, - {"[::1]:8200", true}, + {"127.1.1.1", true}, // Non-localhost {"192.168.1.1", false}, {"vault.example.com", false}, - {"vault.example.com:8200", false}, {"172.16.0.1", false}, {"example.com", false}, - {"[2001:db8::1]", false}, {"2001:db8::1", false}, } for _, tc := range testCases { @@ -348,48 +341,3 @@ func TestValidateSecretsAddr_NoHost(t *testing.T) { }) } } - -func TestStripPort(t *testing.T) { - t.Parallel() - - testCases := map[string]struct { - input string - expected string - }{ - // IPv6 with brackets - "[::1]:8200": {input: "[::1]:8200", expected: "::1"}, - "[::1]": {input: "[::1]", expected: "::1"}, - "[fe80::1]:9090": {input: "[fe80::1]:9090", expected: "fe80::1"}, - "[2001:db8::1]:443": {input: "[2001:db8::1]:443", expected: "2001:db8::1"}, - - // IPv4 with port - "192.168.1.1:8200": {input: "192.168.1.1:8200", expected: "192.168.1.1"}, - "127.0.0.1:9090": {input: "127.0.0.1:9090", expected: "127.0.0.1"}, - - // Hostname with port - "localhost:8200": {input: "localhost:8200", expected: "localhost"}, - "vault.example.com:8200": {input: "vault.example.com:8200", expected: "vault.example.com"}, - - // No port (returned as-is by final return statement on line 407) - "localhost": {input: "localhost", expected: "localhost"}, - "192.168.1.1": {input: "192.168.1.1", expected: "192.168.1.1"}, - "127.0.0.1": {input: "127.0.0.1", expected: "127.0.0.1"}, - "vault.example.com": {input: "vault.example.com", expected: "vault.example.com"}, - "::1": {input: "::1", expected: "::1"}, // Unbracketed IPv6, no port - "fe80::1": {input: "fe80::1", expected: "fe80::1"}, // Unbracketed IPv6 with multiple colons - "2001:db8::1": {input: "2001:db8::1", expected: "2001:db8::1"}, // Full IPv6, no port - - // Edge cases - "[::1]incomplete": {input: "[::1]incomplete", expected: "[::1]incomplete"}, // Malformed: extra suffix after closing bracket returned as-is - "localhost:": {input: "localhost:", expected: "localhost"}, // Trailing colon - } - - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - t.Parallel() - - result := stripPort(tc.input) - assert.Equal(t, tc.expected, result, "stripPort(%q) = %q, want %q", tc.input, result, tc.expected) - }) - } -} From 39f5cfa06c0e183becf5829923ab2e3a3a7e4b71 Mon Sep 17 00:00:00 2001 From: Natalie Gaston Date: Thu, 20 Aug 2026 19:50:52 -0700 Subject: [PATCH 3/4] fix: issues after rebase --- config/config.go | 5 +++++ config/config_test.go | 1 - 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/config/config.go b/config/config.go index 574e9f5f5..3ccf717c7 100644 --- a/config/config.go +++ b/config/config.go @@ -529,6 +529,11 @@ func (c *Config) validateSecretsAddr() error { // Enforce HTTPS for non-localhost if parsed.Scheme == "http" && !isLocalhost(hostname) { return ErrSecretsAddrInsecure + } + + return nil +} + // Sentinel errors for port validation. var ( ErrPortNotNumeric = errors.New("HTTP port (HTTP_PORT) must be a decimal integer") diff --git a/config/config_test.go b/config/config_test.go index d1cc63d18..caf21d406 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -6,7 +6,6 @@ import ( "path/filepath" "runtime" "testing" - "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" From ca6afe7ec6c450da5cf58643b4b6694d33785cd9 Mon Sep 17 00:00:00 2001 From: Natalie Gaston Date: Fri, 21 Aug 2026 08:32:59 -0700 Subject: [PATCH 4/4] chore: formatting --- config/config.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/config/config.go b/config/config.go index 3ccf717c7..cc31bcd42 100644 --- a/config/config.go +++ b/config/config.go @@ -8,9 +8,9 @@ import ( "net/url" "os" "path/filepath" - "strings" "runtime" "strconv" + "strings" "time" "github.com/ilyakaznacheev/cleanenv" @@ -24,10 +24,10 @@ var TrayMode bool // Validation errors for configuration. var ( - ErrSecretsAddrInsecure = errors.New("SECRETS_ADDR must use HTTPS for non-localhost addresses") - ErrSecretsAddrInvalid = errors.New("invalid SECRETS_ADDR") - ErrSecretsAddrMissingScheme = errors.New("SECRETS_ADDR missing scheme (use http:// or https://)") - ErrSecretsAddrNoHost = errors.New("SECRETS_ADDR contains no host") + ErrSecretsAddrInsecure = errors.New("SECRETS_ADDR must use HTTPS for non-localhost addresses") + ErrSecretsAddrInvalid = errors.New("invalid SECRETS_ADDR") + ErrSecretsAddrMissingScheme = errors.New("SECRETS_ADDR missing scheme (use http:// or https://)") + ErrSecretsAddrNoHost = errors.New("SECRETS_ADDR contains no host") ErrJWTExpirationInvalid = errors.New("config: auth.jwtExpiration must be at least 1 minute (e.g. 24h) — very short expirations render tokens unusable") ErrRedirectionJWTExpirationInvalid = errors.New("config: auth.redirectionJWTExpiration must be at least 1 minute (e.g. 5m) — very short expirations render redirection tokens unusable") )