Skip to content

Commit b33cf66

Browse files
JAORMXclaudedmjb
authored
Add build environment configuration and validation (#2740)
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 <[email protected]> Co-authored-by: Don Browne <[email protected]>
1 parent ccd5812 commit b33cf66

File tree

6 files changed

+562
-0
lines changed

6 files changed

+562
-0
lines changed

‎pkg/config/buildenv.go‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
package config
2+
3+
import (
4+
"fmt"
5+
"regexp"
6+
"strings"
7+
)
8+
9+
// Build environment validation constants
10+
const (
11+
errInvalidEnvKeyFormat = "invalid environment variable name: %s (must match pattern %s)"
12+
errReservedEnvKey = "environment variable name %s is reserved and cannot be overridden"
13+
errInvalidEnvValueChars = "environment variable value contains potentially dangerous characters"
14+
)
15+
16+
// envKeyPattern matches valid environment variable names.
17+
// Must start with uppercase letter, followed by uppercase letters, numbers, or underscores.
18+
var envKeyPattern = regexp.MustCompile(`^[A-Z][A-Z0-9_]*$`)
19+
20+
// reservedEnvKeys lists environment variables that cannot be overridden for security reasons.
21+
var reservedEnvKeys = map[string]bool{
22+
"PATH": true,
23+
"HOME": true,
24+
"USER": true,
25+
"SHELL": true,
26+
"PWD": true,
27+
"HOSTNAME": true,
28+
"TERM": true,
29+
"LANG": true,
30+
"LC_ALL": true,
31+
"LD_PRELOAD": true,
32+
"LD_LIBRARY_PATH": true,
33+
}
34+
35+
// ValidateBuildEnvKey validates that an environment variable key follows the required pattern
36+
// and is not a reserved variable.
37+
func ValidateBuildEnvKey(key string) error {
38+
if !envKeyPattern.MatchString(key) {
39+
return fmt.Errorf(errInvalidEnvKeyFormat, key, "^[A-Z][A-Z0-9_]*$")
40+
}
41+
42+
if reservedEnvKeys[key] {
43+
return fmt.Errorf(errReservedEnvKey, key)
44+
}
45+
46+
return nil
47+
}
48+
49+
// ValidateBuildEnvValue validates that an environment variable value does not contain
50+
// potentially dangerous characters that could enable shell injection in Dockerfiles.
51+
func ValidateBuildEnvValue(value string) error {
52+
// Check for shell metacharacters that could enable injection
53+
dangerousPatterns := []string{
54+
"`", // Command substitution
55+
"$(", // Command substitution
56+
"${", // Variable expansion (could be used for injection)
57+
"\\", // Escape sequences
58+
"\n", // Newlines could break Dockerfile syntax
59+
"\r", // Carriage returns
60+
"\"", // Double quotes could break ENV syntax
61+
";", // Command separator
62+
"&&", // Command chaining
63+
"||", // Command chaining
64+
"|", // Pipe
65+
">", // Redirection
66+
"<", // Redirection
67+
}
68+
69+
for _, pattern := range dangerousPatterns {
70+
if strings.Contains(value, pattern) {
71+
return fmt.Errorf("%s: contains '%s'", errInvalidEnvValueChars, pattern)
72+
}
73+
}
74+
75+
return nil
76+
}
77+
78+
// ValidateBuildEnvEntry validates both the key and value of a build environment variable.
79+
func ValidateBuildEnvEntry(key, value string) error {
80+
if err := ValidateBuildEnvKey(key); err != nil {
81+
return err
82+
}
83+
return ValidateBuildEnvValue(value)
84+
}
85+
86+
// setBuildEnv is a helper function that validates and sets a build environment variable.
87+
func setBuildEnv(p Provider, key, value string) error {
88+
if err := ValidateBuildEnvEntry(key, value); err != nil {
89+
return err
90+
}
91+
92+
return p.UpdateConfig(func(c *Config) {
93+
if c.BuildEnv == nil {
94+
c.BuildEnv = make(map[string]string)
95+
}
96+
c.BuildEnv[key] = value
97+
})
98+
}
99+
100+
// getBuildEnv is a helper function that retrieves a build environment variable.
101+
func getBuildEnv(p Provider, key string) (value string, exists bool) {
102+
config := p.GetConfig()
103+
if config.BuildEnv == nil {
104+
return "", false
105+
}
106+
value, exists = config.BuildEnv[key]
107+
return value, exists
108+
}
109+
110+
// getAllBuildEnv is a helper function that retrieves all build environment variables.
111+
func getAllBuildEnv(p Provider) map[string]string {
112+
config := p.GetConfig()
113+
if config.BuildEnv == nil {
114+
return make(map[string]string)
115+
}
116+
// Return a copy to prevent external modifications
117+
result := make(map[string]string, len(config.BuildEnv))
118+
for k, v := range config.BuildEnv {
119+
result[k] = v
120+
}
121+
return result
122+
}
123+
124+
// unsetBuildEnv is a helper function that removes a specific build environment variable.
125+
func unsetBuildEnv(p Provider, key string) error {
126+
return p.UpdateConfig(func(c *Config) {
127+
if c.BuildEnv != nil {
128+
delete(c.BuildEnv, key)
129+
}
130+
})
131+
}
132+
133+
// unsetAllBuildEnv is a helper function that removes all build environment variables.
134+
func unsetAllBuildEnv(p Provider) error {
135+
return p.UpdateConfig(func(c *Config) {
136+
c.BuildEnv = nil
137+
})
138+
}

‎pkg/config/buildenv_test.go‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
package config
2+
3+
import (
4+
"testing"
5+
)
6+
7+
func TestValidateBuildEnvKey(t *testing.T) {
8+
t.Parallel()
9+
10+
tests := []struct {
11+
name string
12+
key string
13+
wantErr bool
14+
}{
15+
// Valid keys
16+
{name: "simple uppercase", key: "NPM_CONFIG_REGISTRY", wantErr: false},
17+
{name: "with numbers", key: "GO111MODULE", wantErr: false},
18+
{name: "single letter", key: "A", wantErr: false},
19+
{name: "all caps with underscore", key: "PIP_INDEX_URL", wantErr: false},
20+
{name: "uv default index", key: "UV_DEFAULT_INDEX", wantErr: false},
21+
{name: "goproxy", key: "GOPROXY", wantErr: false},
22+
{name: "goprivate", key: "GOPRIVATE", wantErr: false},
23+
{name: "node options", key: "NODE_OPTIONS", wantErr: false},
24+
25+
// Invalid keys - pattern mismatch
26+
{name: "lowercase", key: "npm_config_registry", wantErr: true},
27+
{name: "starts with number", key: "1VAR", wantErr: true},
28+
{name: "starts with underscore", key: "_VAR", wantErr: true},
29+
{name: "contains lowercase", key: "NPM_config_REGISTRY", wantErr: true},
30+
{name: "contains hyphen", key: "NPM-CONFIG", wantErr: true},
31+
{name: "contains space", key: "NPM CONFIG", wantErr: true},
32+
{name: "empty string", key: "", wantErr: true},
33+
{name: "contains dot", key: "NPM.CONFIG", wantErr: true},
34+
35+
// Reserved keys
36+
{name: "reserved PATH", key: "PATH", wantErr: true},
37+
{name: "reserved HOME", key: "HOME", wantErr: true},
38+
{name: "reserved USER", key: "USER", wantErr: true},
39+
{name: "reserved SHELL", key: "SHELL", wantErr: true},
40+
{name: "reserved LD_PRELOAD", key: "LD_PRELOAD", wantErr: true},
41+
{name: "reserved LD_LIBRARY_PATH", key: "LD_LIBRARY_PATH", wantErr: true},
42+
}
43+
44+
for _, tt := range tests {
45+
t.Run(tt.name, func(t *testing.T) {
46+
t.Parallel()
47+
48+
err := ValidateBuildEnvKey(tt.key)
49+
if (err != nil) != tt.wantErr {
50+
t.Errorf("ValidateBuildEnvKey(%q) error = %v, wantErr %v", tt.key, err, tt.wantErr)
51+
}
52+
})
53+
}
54+
}
55+
56+
func TestValidateBuildEnvValue(t *testing.T) {
57+
t.Parallel()
58+
59+
tests := []struct {
60+
name string
61+
value string
62+
wantErr bool
63+
}{
64+
// Valid values
65+
{name: "simple URL", value: "https://npm.corp.example.com", wantErr: false},
66+
{name: "URL with path", value: "https://artifactory.corp.example.com/api/npm/npm-remote/", wantErr: false},
67+
{name: "URL with port", value: "https://registry.example.com:8443", wantErr: false},
68+
{name: "simple string", value: "latest", wantErr: false},
69+
{name: "comma-separated", value: "github.com/myorg/*,gitlab.mycompany.com/*", wantErr: false},
70+
{name: "memory limit", value: "--max-old-space-size=4096", wantErr: false},
71+
{name: "empty string", value: "", wantErr: false},
72+
{name: "with equals sign", value: "key=value", wantErr: false},
73+
{name: "with single quotes", value: "it's fine", wantErr: false},
74+
75+
// Invalid values - dangerous characters
76+
{name: "backtick command substitution", value: "`whoami`", wantErr: true},
77+
{name: "dollar paren command substitution", value: "$(whoami)", wantErr: true},
78+
{name: "variable expansion", value: "${HOME}", wantErr: true},
79+
{name: "backslash escape", value: "test\\nvalue", wantErr: true},
80+
{name: "newline", value: "test\nvalue", wantErr: true},
81+
{name: "carriage return", value: "test\rvalue", wantErr: true},
82+
{name: "double quote", value: "test\"value", wantErr: true},
83+
{name: "semicolon", value: "test;whoami", wantErr: true},
84+
{name: "and chain", value: "test&&whoami", wantErr: true},
85+
{name: "or chain", value: "test||whoami", wantErr: true},
86+
{name: "pipe", value: "test|whoami", wantErr: true},
87+
{name: "output redirect", value: "test>file", wantErr: true},
88+
{name: "input redirect", value: "test<file", wantErr: true},
89+
}
90+
91+
for _, tt := range tests {
92+
t.Run(tt.name, func(t *testing.T) {
93+
t.Parallel()
94+
95+
err := ValidateBuildEnvValue(tt.value)
96+
if (err != nil) != tt.wantErr {
97+
t.Errorf("ValidateBuildEnvValue(%q) error = %v, wantErr %v", tt.value, err, tt.wantErr)
98+
}
99+
})
100+
}
101+
}
102+
103+
func TestValidateBuildEnvEntry(t *testing.T) {
104+
t.Parallel()
105+
106+
tests := []struct {
107+
name string
108+
key string
109+
value string
110+
wantErr bool
111+
}{
112+
{
113+
name: "valid entry",
114+
key: "NPM_CONFIG_REGISTRY",
115+
value: "https://npm.corp.example.com",
116+
wantErr: false,
117+
},
118+
{
119+
name: "invalid key",
120+
key: "npm_config_registry",
121+
value: "https://npm.corp.example.com",
122+
wantErr: true,
123+
},
124+
{
125+
name: "invalid value",
126+
key: "NPM_CONFIG_REGISTRY",
127+
value: "$(whoami)",
128+
wantErr: true,
129+
},
130+
{
131+
name: "reserved key",
132+
key: "PATH",
133+
value: "/usr/local/bin",
134+
wantErr: true,
135+
},
136+
{
137+
name: "both invalid",
138+
key: "path",
139+
value: "$(whoami)",
140+
wantErr: true,
141+
},
142+
}
143+
144+
for _, tt := range tests {
145+
t.Run(tt.name, func(t *testing.T) {
146+
t.Parallel()
147+
148+
err := ValidateBuildEnvEntry(tt.key, tt.value)
149+
if (err != nil) != tt.wantErr {
150+
t.Errorf("ValidateBuildEnvEntry(%q, %q) error = %v, wantErr %v", tt.key, tt.value, err, tt.wantErr)
151+
}
152+
})
153+
}
154+
}

‎pkg/config/config.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ type Config struct {
3434
OTEL OpenTelemetryConfig `yaml:"otel,omitempty"`
3535
DefaultGroupMigration bool `yaml:"default_group_migration,omitempty"`
3636
DisableUsageMetrics bool `yaml:"disable_usage_metrics,omitempty"`
37+
BuildEnv map[string]string `yaml:"build_env,omitempty"`
3738
}
3839

3940
// Secrets contains the settings for secrets management.

0 commit comments

Comments
 (0)