From 826d2be5a42421e77246117a38fb588b1c0d421d Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Tue, 25 Nov 2025 20:56:48 -0600 Subject: [PATCH] Add build environment configuration and validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add configuration model and validation for build-time environment variables that will be injected into protocol build Dockerfiles. Changes: - Add BuildEnv map[string]string field to Config struct - Add validation functions for env var keys (uppercase, no reserved keys) - Add validation functions for env var values (no shell metacharacters) - Extend Provider interface with SetBuildEnv, GetBuildEnv, GetAllBuildEnv, UnsetBuildEnv, and UnsetAllBuildEnv methods - Implement for DefaultProvider, PathProvider, and KubernetesProvider - Add comprehensive tests for validation and provider operations This is the foundation for THV-2732 custom package registry support. The CLI commands and template integration will follow in a separate PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- pkg/config/buildenv.go | 138 ++++++++++++++++++++++++++ pkg/config/buildenv_test.go | 154 ++++++++++++++++++++++++++++++ pkg/config/config.go | 1 + pkg/config/interface.go | 82 ++++++++++++++++ pkg/config/interface_test.go | 116 ++++++++++++++++++++++ pkg/config/mocks/mock_provider.go | 71 ++++++++++++++ 6 files changed, 562 insertions(+) create mode 100644 pkg/config/buildenv.go create mode 100644 pkg/config/buildenv_test.go diff --git a/pkg/config/buildenv.go b/pkg/config/buildenv.go new file mode 100644 index 000000000..0e89f875a --- /dev/null +++ b/pkg/config/buildenv.go @@ -0,0 +1,138 @@ +package config + +import ( + "fmt" + "regexp" + "strings" +) + +// Build environment validation constants +const ( + errInvalidEnvKeyFormat = "invalid environment variable name: %s (must match pattern %s)" + errReservedEnvKey = "environment variable name %s is reserved and cannot be overridden" + errInvalidEnvValueChars = "environment variable value contains potentially dangerous characters" +) + +// envKeyPattern matches valid environment variable names. +// Must start with uppercase letter, followed by uppercase letters, numbers, or underscores. +var envKeyPattern = regexp.MustCompile(`^[A-Z][A-Z0-9_]*$`) + +// reservedEnvKeys lists environment variables that cannot be overridden for security reasons. +var reservedEnvKeys = map[string]bool{ + "PATH": true, + "HOME": true, + "USER": true, + "SHELL": true, + "PWD": true, + "HOSTNAME": true, + "TERM": true, + "LANG": true, + "LC_ALL": true, + "LD_PRELOAD": true, + "LD_LIBRARY_PATH": true, +} + +// ValidateBuildEnvKey validates that an environment variable key follows the required pattern +// and is not a reserved variable. +func ValidateBuildEnvKey(key string) error { + if !envKeyPattern.MatchString(key) { + return fmt.Errorf(errInvalidEnvKeyFormat, key, "^[A-Z][A-Z0-9_]*$") + } + + if reservedEnvKeys[key] { + return fmt.Errorf(errReservedEnvKey, key) + } + + return nil +} + +// ValidateBuildEnvValue validates that an environment variable value does not contain +// potentially dangerous characters that could enable shell injection in Dockerfiles. +func ValidateBuildEnvValue(value string) error { + // Check for shell metacharacters that could enable injection + dangerousPatterns := []string{ + "`", // Command substitution + "$(", // Command substitution + "${", // Variable expansion (could be used for injection) + "\\", // Escape sequences + "\n", // Newlines could break Dockerfile syntax + "\r", // Carriage returns + "\"", // Double quotes could break ENV syntax + ";", // Command separator + "&&", // Command chaining + "||", // Command chaining + "|", // Pipe + ">", // Redirection + "<", // Redirection + } + + for _, pattern := range dangerousPatterns { + if strings.Contains(value, pattern) { + return fmt.Errorf("%s: contains '%s'", errInvalidEnvValueChars, pattern) + } + } + + return nil +} + +// ValidateBuildEnvEntry validates both the key and value of a build environment variable. +func ValidateBuildEnvEntry(key, value string) error { + if err := ValidateBuildEnvKey(key); err != nil { + return err + } + return ValidateBuildEnvValue(value) +} + +// setBuildEnv is a helper function that validates and sets a build environment variable. +func setBuildEnv(p Provider, key, value string) error { + if err := ValidateBuildEnvEntry(key, value); err != nil { + return err + } + + return p.UpdateConfig(func(c *Config) { + if c.BuildEnv == nil { + c.BuildEnv = make(map[string]string) + } + c.BuildEnv[key] = value + }) +} + +// getBuildEnv is a helper function that retrieves a build environment variable. +func getBuildEnv(p Provider, key string) (value string, exists bool) { + config := p.GetConfig() + if config.BuildEnv == nil { + return "", false + } + value, exists = config.BuildEnv[key] + return value, exists +} + +// getAllBuildEnv is a helper function that retrieves all build environment variables. +func getAllBuildEnv(p Provider) map[string]string { + config := p.GetConfig() + if config.BuildEnv == nil { + return make(map[string]string) + } + // Return a copy to prevent external modifications + result := make(map[string]string, len(config.BuildEnv)) + for k, v := range config.BuildEnv { + result[k] = v + } + return result +} + +// unsetBuildEnv is a helper function that removes a specific build environment variable. +func unsetBuildEnv(p Provider, key string) error { + return p.UpdateConfig(func(c *Config) { + if c.BuildEnv != nil { + delete(c.BuildEnv, key) + } + }) +} + +// unsetAllBuildEnv is a helper function that removes all build environment variables. +func unsetAllBuildEnv(p Provider) error { + return p.UpdateConfig(func(c *Config) { + c.BuildEnv = nil + }) +} diff --git a/pkg/config/buildenv_test.go b/pkg/config/buildenv_test.go new file mode 100644 index 000000000..76a774c68 --- /dev/null +++ b/pkg/config/buildenv_test.go @@ -0,0 +1,154 @@ +package config + +import ( + "testing" +) + +func TestValidateBuildEnvKey(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + key string + wantErr bool + }{ + // Valid keys + {name: "simple uppercase", key: "NPM_CONFIG_REGISTRY", wantErr: false}, + {name: "with numbers", key: "GO111MODULE", wantErr: false}, + {name: "single letter", key: "A", wantErr: false}, + {name: "all caps with underscore", key: "PIP_INDEX_URL", wantErr: false}, + {name: "uv default index", key: "UV_DEFAULT_INDEX", wantErr: false}, + {name: "goproxy", key: "GOPROXY", wantErr: false}, + {name: "goprivate", key: "GOPRIVATE", wantErr: false}, + {name: "node options", key: "NODE_OPTIONS", wantErr: false}, + + // Invalid keys - pattern mismatch + {name: "lowercase", key: "npm_config_registry", wantErr: true}, + {name: "starts with number", key: "1VAR", wantErr: true}, + {name: "starts with underscore", key: "_VAR", wantErr: true}, + {name: "contains lowercase", key: "NPM_config_REGISTRY", wantErr: true}, + {name: "contains hyphen", key: "NPM-CONFIG", wantErr: true}, + {name: "contains space", key: "NPM CONFIG", wantErr: true}, + {name: "empty string", key: "", wantErr: true}, + {name: "contains dot", key: "NPM.CONFIG", wantErr: true}, + + // Reserved keys + {name: "reserved PATH", key: "PATH", wantErr: true}, + {name: "reserved HOME", key: "HOME", wantErr: true}, + {name: "reserved USER", key: "USER", wantErr: true}, + {name: "reserved SHELL", key: "SHELL", wantErr: true}, + {name: "reserved LD_PRELOAD", key: "LD_PRELOAD", wantErr: true}, + {name: "reserved LD_LIBRARY_PATH", key: "LD_LIBRARY_PATH", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := ValidateBuildEnvKey(tt.key) + if (err != nil) != tt.wantErr { + t.Errorf("ValidateBuildEnvKey(%q) error = %v, wantErr %v", tt.key, err, tt.wantErr) + } + }) + } +} + +func TestValidateBuildEnvValue(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + value string + wantErr bool + }{ + // Valid values + {name: "simple URL", value: "https://npm.corp.example.com", wantErr: false}, + {name: "URL with path", value: "https://artifactory.corp.example.com/api/npm/npm-remote/", wantErr: false}, + {name: "URL with port", value: "https://registry.example.com:8443", wantErr: false}, + {name: "simple string", value: "latest", wantErr: false}, + {name: "comma-separated", value: "github.com/myorg/*,gitlab.mycompany.com/*", wantErr: false}, + {name: "memory limit", value: "--max-old-space-size=4096", wantErr: false}, + {name: "empty string", value: "", wantErr: false}, + {name: "with equals sign", value: "key=value", wantErr: false}, + {name: "with single quotes", value: "it's fine", wantErr: false}, + + // Invalid values - dangerous characters + {name: "backtick command substitution", value: "`whoami`", wantErr: true}, + {name: "dollar paren command substitution", value: "$(whoami)", wantErr: true}, + {name: "variable expansion", value: "${HOME}", wantErr: true}, + {name: "backslash escape", value: "test\\nvalue", wantErr: true}, + {name: "newline", value: "test\nvalue", wantErr: true}, + {name: "carriage return", value: "test\rvalue", wantErr: true}, + {name: "double quote", value: "test\"value", wantErr: true}, + {name: "semicolon", value: "test;whoami", wantErr: true}, + {name: "and chain", value: "test&&whoami", wantErr: true}, + {name: "or chain", value: "test||whoami", wantErr: true}, + {name: "pipe", value: "test|whoami", wantErr: true}, + {name: "output redirect", value: "test>file", wantErr: true}, + {name: "input redirect", value: "test