From 5a031f5d1bf03683d32bf2d8ff2cc698fff689fa Mon Sep 17 00:00:00 2001 From: Elias Schneider Date: Thu, 7 Aug 2025 20:41:00 +0200 Subject: [PATCH] refactor: use reflection to mark file based env variables (#815) --- backend/internal/cmds/key_rotate_test.go | 2 +- backend/internal/common/env_config.go | 171 +++++++++++---------- backend/internal/common/env_config_test.go | 119 +++++++++++++- 3 files changed, 210 insertions(+), 82 deletions(-) diff --git a/backend/internal/cmds/key_rotate_test.go b/backend/internal/cmds/key_rotate_test.go index e7ecd47f..c6528019 100644 --- a/backend/internal/cmds/key_rotate_test.go +++ b/backend/internal/cmds/key_rotate_test.go @@ -141,7 +141,7 @@ func testKeyRotateWithDatabaseStorage(t *testing.T, flags keyRotateFlags, wantEr // Set up database storage config envConfig := &common.EnvConfigSchema{ KeysStorage: "database", - EncryptionKey: "test-encryption-key-characters-long", + EncryptionKey: []byte("test-encryption-key-characters-long"), } // Create test database diff --git a/backend/internal/common/env_config.go b/backend/internal/common/env_config.go index dbd445e2..476c5618 100644 --- a/backend/internal/common/env_config.go +++ b/backend/internal/common/env_config.go @@ -6,30 +6,13 @@ import ( "log/slog" "net/url" "os" + "reflect" "strings" "github.com/caarlos0/env/v11" _ "github.com/joho/godotenv/autoload" ) -func resolveStringOrFile(directValue string, filePath string, varName string, trim bool) (string, error) { - if directValue != "" { - return directValue, nil - } - if filePath != "" { - content, err := os.ReadFile(filePath) - if err != nil { - return "", fmt.Errorf("failed to read secret '%s' from file '%s': %w", varName, filePath, err) - } - - if trim { - return strings.TrimSpace(string(content)), nil - } - return string(content), nil - } - return "", nil -} - type DbProvider string const ( @@ -47,31 +30,28 @@ const ( ) type EnvConfigSchema struct { - AppEnv string `env:"APP_ENV"` - AppURL string `env:"APP_URL"` - DbProvider DbProvider `env:"DB_PROVIDER"` - DbConnectionString string `env:"DB_CONNECTION_STRING"` - DbConnectionStringFile string `env:"DB_CONNECTION_STRING_FILE"` - UploadPath string `env:"UPLOAD_PATH"` - KeysPath string `env:"KEYS_PATH"` - KeysStorage string `env:"KEYS_STORAGE"` - EncryptionKey string `env:"ENCRYPTION_KEY"` - EncryptionKeyFile string `env:"ENCRYPTION_KEY_FILE"` - Port string `env:"PORT"` - Host string `env:"HOST"` - UnixSocket string `env:"UNIX_SOCKET"` - UnixSocketMode string `env:"UNIX_SOCKET_MODE"` - MaxMindLicenseKey string `env:"MAXMIND_LICENSE_KEY"` - MaxMindLicenseKeyFile string `env:"MAXMIND_LICENSE_KEY_FILE"` - GeoLiteDBPath string `env:"GEOLITE_DB_PATH"` - GeoLiteDBUrl string `env:"GEOLITE_DB_URL"` - LocalIPv6Ranges string `env:"LOCAL_IPV6_RANGES"` - UiConfigDisabled bool `env:"UI_CONFIG_DISABLED"` - MetricsEnabled bool `env:"METRICS_ENABLED"` - TracingEnabled bool `env:"TRACING_ENABLED"` - LogJSON bool `env:"LOG_JSON"` - TrustProxy bool `env:"TRUST_PROXY"` - AnalyticsDisabled bool `env:"ANALYTICS_DISABLED"` + AppEnv string `env:"APP_ENV"` + AppURL string `env:"APP_URL"` + DbProvider DbProvider `env:"DB_PROVIDER"` + DbConnectionString string `env:"DB_CONNECTION_STRING" options:"file"` + UploadPath string `env:"UPLOAD_PATH"` + KeysPath string `env:"KEYS_PATH"` + KeysStorage string `env:"KEYS_STORAGE"` + EncryptionKey []byte `env:"ENCRYPTION_KEY" options:"file"` + Port string `env:"PORT"` + Host string `env:"HOST"` + UnixSocket string `env:"UNIX_SOCKET"` + UnixSocketMode string `env:"UNIX_SOCKET_MODE"` + MaxMindLicenseKey string `env:"MAXMIND_LICENSE_KEY" options:"file"` + GeoLiteDBPath string `env:"GEOLITE_DB_PATH"` + GeoLiteDBUrl string `env:"GEOLITE_DB_URL"` + LocalIPv6Ranges string `env:"LOCAL_IPV6_RANGES"` + UiConfigDisabled bool `env:"UI_CONFIG_DISABLED"` + MetricsEnabled bool `env:"METRICS_ENABLED"` + TracingEnabled bool `env:"TRACING_ENABLED"` + LogJSON bool `env:"LOG_JSON"` + TrustProxy bool `env:"TRUST_PROXY"` + AnalyticsDisabled bool `env:"ANALYTICS_DISABLED"` } var EnvConfig = defaultConfig() @@ -92,7 +72,7 @@ func defaultConfig() EnvConfigSchema { UploadPath: "data/uploads", KeysPath: "data/keys", KeysStorage: "", // "database" or "file" - EncryptionKey: "", + EncryptionKey: nil, AppURL: "http://localhost:1411", Port: "1411", Host: "0.0.0.0", @@ -111,33 +91,23 @@ func defaultConfig() EnvConfigSchema { } func parseEnvConfig() error { - err := env.ParseWithOptions(&EnvConfig, env.Options{}) + parsers := map[reflect.Type]env.ParserFunc{ + reflect.TypeOf([]byte{}): func(value string) (interface{}, error) { + return []byte(value), nil + }, + } + + err := env.ParseWithOptions(&EnvConfig, env.Options{ + FuncMap: parsers, + }) if err != nil { return fmt.Errorf("error parsing env config: %w", err) } - // Resolve string/file environment variables - EnvConfig.DbConnectionString, err = resolveStringOrFile( - EnvConfig.DbConnectionString, - EnvConfig.DbConnectionStringFile, - "DB_CONNECTION_STRING", - true, - ) + err = resolveFileBasedEnvVariables(&EnvConfig) if err != nil { return err } - EnvConfig.DbConnectionStringFile = "" - - EnvConfig.MaxMindLicenseKey, err = resolveStringOrFile( - EnvConfig.MaxMindLicenseKey, - EnvConfig.MaxMindLicenseKeyFile, - "MAXMIND_LICENSE_KEY", - true, - ) - if err != nil { - return err - } - EnvConfig.MaxMindLicenseKeyFile = "" // Validate the environment variables switch EnvConfig.DbProvider { @@ -166,23 +136,9 @@ func parseEnvConfig() error { case "": EnvConfig.KeysStorage = "file" case "database": - // Resolve encryption key using the same pattern - encryptionKey, err := resolveStringOrFile( - EnvConfig.EncryptionKey, - EnvConfig.EncryptionKeyFile, - "ENCRYPTION_KEY", - // Do not trim spaces because the file should be interpreted as binary - false, - ) - if err != nil { - return err + if EnvConfig.EncryptionKey == nil { + return errors.New("ENCRYPTION_KEY must be non-empty when KEYS_STORAGE is database") } - if encryptionKey == "" { - return errors.New("ENCRYPTION_KEY or ENCRYPTION_KEY_FILE must be non-empty when KEYS_STORAGE is database") - } - // Update the config with resolved value - EnvConfig.EncryptionKey = encryptionKey - EnvConfig.EncryptionKeyFile = "" case "file": // All good, these are valid values default: @@ -191,3 +147,58 @@ func parseEnvConfig() error { return nil } + +// resolveFileBasedEnvVariables uses reflection to automatically resolve file-based secrets +func resolveFileBasedEnvVariables(config *EnvConfigSchema) error { + val := reflect.ValueOf(config).Elem() + typ := val.Type() + + for i := 0; i < val.NumField(); i++ { + field := val.Field(i) + fieldType := typ.Field(i) + + // Only process string and []byte fields + isString := field.Kind() == reflect.String + isByteSlice := field.Kind() == reflect.Slice && field.Type().Elem().Kind() == reflect.Uint8 + if !isString && !isByteSlice { + continue + } + + // Only process fields with the "options" tag set to "file" + optionsTag := fieldType.Tag.Get("options") + if optionsTag != "file" { + continue + } + + // Only process fields with the "env" tag + envTag := fieldType.Tag.Get("env") + if envTag == "" { + continue + } + + envVarName := envTag + if commaIndex := len(envTag); commaIndex > 0 { + envVarName = envTag[:commaIndex] + } + + // If the file environment variable is not set, skip + envVarFileName := envVarName + "_FILE" + envVarFileValue := os.Getenv(envVarFileName) + if envVarFileValue == "" { + continue + } + + fileContent, err := os.ReadFile(envVarFileValue) + if err != nil { + return fmt.Errorf("failed to read file for env var %s: %w", envVarFileName, err) + } + + if isString { + field.SetString(strings.TrimSpace(string(fileContent))) + } else { + field.SetBytes(fileContent) + } + } + + return nil +} diff --git a/backend/internal/common/env_config_test.go b/backend/internal/common/env_config_test.go index 024eb4c4..ffef2852 100644 --- a/backend/internal/common/env_config_test.go +++ b/backend/internal/common/env_config_test.go @@ -1,6 +1,7 @@ package common import ( + "os" "testing" "github.com/stretchr/testify/assert" @@ -110,7 +111,7 @@ func TestParseEnvConfig(t *testing.T) { err := parseEnvConfig() require.Error(t, err) - assert.ErrorContains(t, err, "ENCRYPTION_KEY or ENCRYPTION_KEY_FILE must be non-empty") + assert.ErrorContains(t, err, "ENCRYPTION_KEY must be non-empty when KEYS_STORAGE is database") }) t.Run("should accept valid KEYS_STORAGE values", func(t *testing.T) { @@ -186,3 +187,119 @@ func TestParseEnvConfig(t *testing.T) { assert.Equal(t, "127.0.0.1", EnvConfig.Host) }) } + +func TestResolveFileBasedEnvVariables(t *testing.T) { + // Create temporary directory for test files + tempDir := t.TempDir() + + // Create test files + encryptionKeyFile := tempDir + "/encryption_key.txt" + encryptionKeyContent := "test-encryption-key-123" + err := os.WriteFile(encryptionKeyFile, []byte(encryptionKeyContent), 0600) + require.NoError(t, err) + + dbConnFile := tempDir + "/db_connection.txt" + dbConnContent := "postgres://user:pass@localhost/testdb" + err = os.WriteFile(dbConnFile, []byte(dbConnContent), 0600) + require.NoError(t, err) + + // Create a binary file for testing binary data handling + binaryKeyFile := tempDir + "/binary_key.bin" + binaryKeyContent := []byte{0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F, 0x10} + err = os.WriteFile(binaryKeyFile, binaryKeyContent, 0600) + require.NoError(t, err) + + t.Run("should read file content for fields with options:file tag", func(t *testing.T) { + config := defaultConfig() + + // Set environment variables pointing to files + t.Setenv("ENCRYPTION_KEY_FILE", encryptionKeyFile) + t.Setenv("DB_CONNECTION_STRING_FILE", dbConnFile) + + err := resolveFileBasedEnvVariables(&config) + require.NoError(t, err) + + // Verify file contents were read correctly + assert.Equal(t, []byte(encryptionKeyContent), config.EncryptionKey) + assert.Equal(t, dbConnContent, config.DbConnectionString) + }) + + t.Run("should skip fields without options:file tag", func(t *testing.T) { + config := defaultConfig() + originalAppURL := config.AppURL + + // Set a file for a field that doesn't have options:file tag + t.Setenv("APP_URL_FILE", "/tmp/nonexistent.txt") + + err := resolveFileBasedEnvVariables(&config) + require.NoError(t, err) + + // AppURL should remain unchanged + assert.Equal(t, originalAppURL, config.AppURL) + }) + + t.Run("should skip non-string fields", func(t *testing.T) { + // This test verifies that non-string fields are skipped + // We test this indirectly by ensuring the function doesn't error + // when processing the actual EnvConfigSchema which has bool fields + config := defaultConfig() + + err := resolveFileBasedEnvVariables(&config) + require.NoError(t, err) + }) + + t.Run("should skip when _FILE environment variable is not set", func(t *testing.T) { + config := defaultConfig() + originalEncryptionKey := config.EncryptionKey + + // Don't set ENCRYPTION_KEY_FILE environment variable + + err := resolveFileBasedEnvVariables(&config) + require.NoError(t, err) + + // EncryptionKey should remain unchanged + assert.Equal(t, originalEncryptionKey, config.EncryptionKey) + }) + + t.Run("should handle multiple file-based variables simultaneously", func(t *testing.T) { + config := defaultConfig() + + // Set multiple file environment variables + t.Setenv("ENCRYPTION_KEY_FILE", encryptionKeyFile) + t.Setenv("DB_CONNECTION_STRING_FILE", dbConnFile) + + err := resolveFileBasedEnvVariables(&config) + require.NoError(t, err) + + // All should be resolved correctly + assert.Equal(t, []byte(encryptionKeyContent), config.EncryptionKey) + assert.Equal(t, dbConnContent, config.DbConnectionString) + }) + + t.Run("should handle mixed file and non-file environment variables", func(t *testing.T) { + config := defaultConfig() + + // Set both file and non-file environment variables + t.Setenv("ENCRYPTION_KEY_FILE", encryptionKeyFile) + + err := resolveFileBasedEnvVariables(&config) + require.NoError(t, err) + + // File-based should be resolved, others should remain as set by env parser + assert.Equal(t, []byte(encryptionKeyContent), config.EncryptionKey) + assert.Equal(t, "http://localhost:1411", config.AppURL) + }) + + t.Run("should handle binary data correctly", func(t *testing.T) { + config := defaultConfig() + + // Set environment variable pointing to binary file + t.Setenv("ENCRYPTION_KEY_FILE", binaryKeyFile) + + err := resolveFileBasedEnvVariables(&config) + require.NoError(t, err) + + // Verify binary data was read correctly without corruption + assert.Equal(t, binaryKeyContent, config.EncryptionKey) + }) +}