diff --git a/cmd/thv/app/config_buildenv.go b/cmd/thv/app/config_buildenv.go index b6622626d..8dbfb9033 100644 --- a/cmd/thv/app/config_buildenv.go +++ b/cmd/thv/app/config_buildenv.go @@ -1,20 +1,24 @@ package app import ( + "context" "fmt" "sort" "github.com/spf13/cobra" "github.com/stacklok/toolhive/pkg/config" + "github.com/stacklok/toolhive/pkg/secrets" ) var ( unsetBuildEnvAll bool + fromSecret bool + fromEnv bool ) var setBuildEnvCmd = &cobra.Command{ - Use: "set-build-env ", + Use: "set-build-env [value]", Short: "Set a build environment variable for protocol builds", Long: `Set a build environment variable that will be injected into Dockerfiles during protocol builds (npx://, uvx://, go://). This is useful for configuring @@ -25,6 +29,11 @@ Environment variable names must: - Contain only uppercase letters, numbers, and underscores - Not be a reserved system variable (PATH, HOME, etc.) +You can set the value in three ways: +1. Directly: thv config set-build-env KEY value +2. From a ToolHive secret: thv config set-build-env KEY --from-secret secret-name +3. From shell environment: thv config set-build-env KEY --from-env + Common use cases: - NPM_CONFIG_REGISTRY: Custom npm registry URL - PIP_INDEX_URL: Custom PyPI index URL @@ -34,10 +43,9 @@ Common use cases: Examples: thv config set-build-env NPM_CONFIG_REGISTRY https://npm.corp.example.com - thv config set-build-env PIP_INDEX_URL https://pypi.corp.example.com/simple - thv config set-build-env GOPROXY https://goproxy.corp.example.com - thv config set-build-env GOPRIVATE "github.com/myorg/*"`, - Args: cobra.ExactArgs(2), + thv config set-build-env GITHUB_TOKEN --from-secret github-pat + thv config set-build-env ARTIFACTORY_API_KEY --from-env`, + Args: cobra.RangeArgs(1, 2), RunE: setBuildEnvCmdFunc, } @@ -73,6 +81,23 @@ func init() { configCmd.AddCommand(getBuildEnvCmd) configCmd.AddCommand(unsetBuildEnvCmd) + // Add --from-secret and --from-env flags to set command + setBuildEnvCmd.Flags().BoolVar( + &fromSecret, + "from-secret", + false, + "Read value from a ToolHive secret at build time (value argument becomes secret name)", + ) + setBuildEnvCmd.Flags().BoolVar( + &fromEnv, + "from-env", + false, + "Read value from shell environment at build time", + ) + + // Make flags mutually exclusive + setBuildEnvCmd.MarkFlagsMutuallyExclusive("from-secret", "from-env") + // Add --all flag to unset command unsetBuildEnvCmd.Flags().BoolVar( &unsetBuildEnvAll, @@ -82,11 +107,79 @@ func init() { ) } +func validateSecretExists(ctx context.Context, secretName string) error { + configProvider := config.NewDefaultProvider() + cfg := configProvider.GetConfig() + + // Check if secrets are set up + if !cfg.Secrets.SetupCompleted { + return secrets.ErrSecretsNotSetup + } + + providerType, err := cfg.Secrets.GetProviderType() + if err != nil { + return fmt.Errorf("failed to get secrets provider type: %w", err) + } + + manager, err := secrets.CreateSecretProvider(providerType) + if err != nil { + return fmt.Errorf("failed to create secrets provider: %w", err) + } + + // Try to get the secret to validate it exists + _, err = manager.GetSecret(ctx, secretName) + if err != nil { + return fmt.Errorf("secret '%s' not found or inaccessible: %w", secretName, err) + } + + return nil +} + func setBuildEnvCmdFunc(_ *cobra.Command, args []string) error { key := args[0] + provider := config.NewDefaultProvider() + + // Handle --from-secret flag + if fromSecret { + if len(args) != 2 { + return fmt.Errorf("secret name is required when using --from-secret") + } + secretName := args[1] + + // Validate that the secret exists + ctx := context.Background() + if err := validateSecretExists(ctx, secretName); err != nil { + return fmt.Errorf("failed to validate secret: %w", err) + } + + if err := provider.SetBuildEnvFromSecret(key, secretName); err != nil { + return fmt.Errorf("failed to set build environment variable from secret: %w", err) + } + + fmt.Printf("Successfully configured build environment variable %s to read from secret: %s\n", key, secretName) + return nil + } + + // Handle --from-env flag + if fromEnv { + if len(args) > 1 { + return fmt.Errorf("value argument should not be provided when using --from-env") + } + + if err := provider.SetBuildEnvFromShell(key); err != nil { + return fmt.Errorf("failed to set build environment variable from shell: %w", err) + } + + fmt.Printf("Successfully configured build environment variable %s to read from shell environment\n", key) + return nil + } + + // Handle literal value + if len(args) != 2 { + return fmt.Errorf("value is required when not using --from-secret or --from-env") + } value := args[1] - provider := config.NewDefaultProvider() if err := provider.SetBuildEnv(key, value); err != nil { return fmt.Errorf("failed to set build environment variable: %w", err) } @@ -95,38 +188,64 @@ func setBuildEnvCmdFunc(_ *cobra.Command, args []string) error { return nil } +// buildEnvEntry represents a build environment variable with its source +type buildEnvEntry struct { + key, value, source string +} + +// getAllBuildEnvEntries collects all build env entries from all sources +func getAllBuildEnvEntries(provider config.Provider) []buildEnvEntry { + var entries []buildEnvEntry + for k, v := range provider.GetAllBuildEnv() { + entries = append(entries, buildEnvEntry{k, v, "literal"}) + } + for k, v := range provider.GetAllBuildEnvFromSecrets() { + entries = append(entries, buildEnvEntry{k, v, "secret"}) + } + for _, k := range provider.GetAllBuildEnvFromShell() { + entries = append(entries, buildEnvEntry{k, "", "shell"}) + } + sort.Slice(entries, func(i, j int) bool { return entries[i].key < entries[j].key }) + return entries +} + +func (e buildEnvEntry) String() string { + switch e.source { + case "secret": + return fmt.Sprintf("%s=", e.key, e.value) + case "shell": + return fmt.Sprintf("%s=", e.key) + default: + return fmt.Sprintf("%s=%s", e.key, e.value) + } +} + func getBuildEnvCmdFunc(_ *cobra.Command, args []string) error { provider := config.NewDefaultProvider() if len(args) == 1 { - // Get specific variable key := args[0] - value, exists := provider.GetBuildEnv(key) - if !exists { + if value, exists := provider.GetBuildEnv(key); exists { + fmt.Printf("%s=%s\n", key, value) + } else if secretName, exists := provider.GetBuildEnvFromSecret(key); exists { + fmt.Printf("%s=\n", key, secretName) + } else if provider.GetBuildEnvFromShell(key) { + fmt.Printf("%s=\n", key) + } else { fmt.Printf("Build environment variable %s is not configured.\n", key) - return nil } - fmt.Printf("%s=%s\n", key, value) return nil } - // Get all variables - envVars := provider.GetAllBuildEnv() - if len(envVars) == 0 { + entries := getAllBuildEnvEntries(provider) + if len(entries) == 0 { fmt.Println("No build environment variables are configured.") return nil } - // Sort keys for consistent output - keys := make([]string, 0, len(envVars)) - for k := range envVars { - keys = append(keys, k) - } - sort.Strings(keys) - fmt.Println("Configured build environment variables:") - for _, k := range keys { - fmt.Printf(" %s=%s\n", k, envVars[k]) + for _, e := range entries { + fmt.Printf(" %s\n", e) } return nil } @@ -135,17 +254,17 @@ func unsetBuildEnvCmdFunc(_ *cobra.Command, args []string) error { provider := config.NewDefaultProvider() if unsetBuildEnvAll { - envVars := provider.GetAllBuildEnv() - if len(envVars) == 0 { + entries := getAllBuildEnvEntries(provider) + if len(entries) == 0 { fmt.Println("No build environment variables are configured.") return nil } - - if err := provider.UnsetAllBuildEnv(); err != nil { - return fmt.Errorf("failed to remove build environment variables: %w", err) + for _, e := range entries { + if err := unsetBuildEnvBySource(provider, e.key, e.source); err != nil { + return err + } } - - fmt.Printf("Successfully removed %d build environment variable(s).\n", len(envVars)) + fmt.Printf("Successfully removed %d build environment variable(s).\n", len(entries)) return nil } @@ -154,16 +273,32 @@ func unsetBuildEnvCmdFunc(_ *cobra.Command, args []string) error { } key := args[0] - _, exists := provider.GetBuildEnv(key) - if !exists { - fmt.Printf("Build environment variable %s is not configured.\n", key) - return nil + if _, exists := provider.GetBuildEnv(key); exists { + return unsetBuildEnvBySource(provider, key, "literal") } - - if err := provider.UnsetBuildEnv(key); err != nil { - return fmt.Errorf("failed to remove build environment variable: %w", err) + if _, exists := provider.GetBuildEnvFromSecret(key); exists { + return unsetBuildEnvBySource(provider, key, "secret") } + if provider.GetBuildEnvFromShell(key) { + return unsetBuildEnvBySource(provider, key, "shell") + } + fmt.Printf("Build environment variable %s is not configured.\n", key) + return nil +} +func unsetBuildEnvBySource(provider config.Provider, key, source string) error { + var err error + switch source { + case "literal": + err = provider.UnsetBuildEnv(key) + case "secret": + err = provider.UnsetBuildEnvFromSecret(key) + case "shell": + err = provider.UnsetBuildEnvFromShell(key) + } + if err != nil { + return fmt.Errorf("failed to remove %s: %w", key, err) + } fmt.Printf("Successfully removed build environment variable: %s\n", key) return nil } diff --git a/docs/cli/thv_config_set-build-env.md b/docs/cli/thv_config_set-build-env.md index 0870f84de..86c37c84d 100644 --- a/docs/cli/thv_config_set-build-env.md +++ b/docs/cli/thv_config_set-build-env.md @@ -24,6 +24,11 @@ Environment variable names must: - Contain only uppercase letters, numbers, and underscores - Not be a reserved system variable (PATH, HOME, etc.) +You can set the value in three ways: +1. Directly: thv config set-build-env KEY value +2. From a ToolHive secret: thv config set-build-env KEY --from-secret secret-name +3. From shell environment: thv config set-build-env KEY --from-env + Common use cases: - NPM_CONFIG_REGISTRY: Custom npm registry URL - PIP_INDEX_URL: Custom PyPI index URL @@ -33,18 +38,19 @@ Common use cases: Examples: thv config set-build-env NPM_CONFIG_REGISTRY https://npm.corp.example.com - thv config set-build-env PIP_INDEX_URL https://pypi.corp.example.com/simple - thv config set-build-env GOPROXY https://goproxy.corp.example.com - thv config set-build-env GOPRIVATE "github.com/myorg/*" + thv config set-build-env GITHUB_TOKEN --from-secret github-pat + thv config set-build-env ARTIFACTORY_API_KEY --from-env ``` -thv config set-build-env [flags] +thv config set-build-env [value] [flags] ``` ### Options ``` - -h, --help help for set-build-env + --from-env Read value from shell environment at build time + --from-secret Read value from a ToolHive secret at build time (value argument becomes secret name) + -h, --help help for set-build-env ``` ### Options inherited from parent commands diff --git a/pkg/config/buildenv.go b/pkg/config/buildenv.go index 0e89f875a..18d9e4fce 100644 --- a/pkg/config/buildenv.go +++ b/pkg/config/buildenv.go @@ -83,12 +83,45 @@ func ValidateBuildEnvEntry(key, value string) error { return ValidateBuildEnvValue(value) } +// checkBuildEnvKeyConflict checks if a key is already configured in another source. +func checkBuildEnvKeyConflict(p Provider, key string) error { + config := p.GetConfig() + + // Check literal values + if config.BuildEnv != nil { + if _, exists := config.BuildEnv[key]; exists { + return fmt.Errorf("key %s already configured as literal value; unset it first with 'thv config unset-build-env %s'", key, key) + } + } + + // Check secret references + if config.BuildEnvFromSecrets != nil { + if _, exists := config.BuildEnvFromSecrets[key]; exists { + return fmt.Errorf("key %s already configured from secret; unset it first", key) + } + } + + // Check shell references + for _, k := range config.BuildEnvFromShell { + if k == key { + return fmt.Errorf("key %s already configured from shell; unset it first", key) + } + } + + return nil +} + // 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 } + // Check for conflicts with other sources + if err := checkBuildEnvKeyConflict(p, key); err != nil { + return err + } + return p.UpdateConfig(func(c *Config) { if c.BuildEnv == nil { c.BuildEnv = make(map[string]string) @@ -136,3 +169,136 @@ func unsetAllBuildEnv(p Provider) error { c.BuildEnv = nil }) } + +// setBuildEnvFromSecret validates and stores a secret reference for a build environment variable. +func setBuildEnvFromSecret(p Provider, key, secretName string) error { + // Validate the key follows the pattern + if err := ValidateBuildEnvKey(key); err != nil { + return err + } + + // Check for conflicts with other sources + if err := checkBuildEnvKeyConflict(p, key); err != nil { + return err + } + + return p.UpdateConfig(func(c *Config) { + if c.BuildEnvFromSecrets == nil { + c.BuildEnvFromSecrets = make(map[string]string) + } + c.BuildEnvFromSecrets[key] = secretName + }) +} + +// getBuildEnvFromSecret retrieves the secret name for a build environment variable. +func getBuildEnvFromSecret(p Provider, key string) (secretName string, exists bool) { + config := p.GetConfig() + if config.BuildEnvFromSecrets == nil { + return "", false + } + secretName, exists = config.BuildEnvFromSecrets[key] + return secretName, exists +} + +// getAllBuildEnvFromSecrets returns all build env secret references. +func getAllBuildEnvFromSecrets(p Provider) map[string]string { + config := p.GetConfig() + if config.BuildEnvFromSecrets == nil { + return make(map[string]string) + } + result := make(map[string]string, len(config.BuildEnvFromSecrets)) + for k, v := range config.BuildEnvFromSecrets { + result[k] = v + } + return result +} + +// unsetBuildEnvFromSecret removes a secret reference. +func unsetBuildEnvFromSecret(p Provider, key string) error { + return p.UpdateConfig(func(c *Config) { + if c.BuildEnvFromSecrets != nil { + delete(c.BuildEnvFromSecrets, key) + } + }) +} + +// setBuildEnvFromShell adds an environment variable name to read from shell at build time. +func setBuildEnvFromShell(p Provider, key string) error { + // Validate the key follows the pattern + if err := ValidateBuildEnvKey(key); err != nil { + return err + } + + // Check if already in the list - skip if so + if getBuildEnvFromShell(p, key) { + return nil // Already exists, nothing to do + } + + // Check for conflicts with other sources (not including shell since we checked above) + if err := checkBuildEnvKeyConflictExcludingShell(p, key); err != nil { + return err + } + + return p.UpdateConfig(func(c *Config) { + c.BuildEnvFromShell = append(c.BuildEnvFromShell, key) + }) +} + +// checkBuildEnvKeyConflictExcludingShell checks if a key is already configured in literal or secret sources. +func checkBuildEnvKeyConflictExcludingShell(p Provider, key string) error { + config := p.GetConfig() + + // Check literal values + if config.BuildEnv != nil { + if _, exists := config.BuildEnv[key]; exists { + return fmt.Errorf("key %s already configured as literal value; unset it first with 'thv config unset-build-env %s'", key, key) + } + } + + // Check secret references + if config.BuildEnvFromSecrets != nil { + if _, exists := config.BuildEnvFromSecrets[key]; exists { + return fmt.Errorf("key %s already configured from secret; unset it first", key) + } + } + + return nil +} + +// getBuildEnvFromShell checks if a key is configured to read from shell. +func getBuildEnvFromShell(p Provider, key string) bool { + config := p.GetConfig() + for _, k := range config.BuildEnvFromShell { + if k == key { + return true + } + } + return false +} + +// getAllBuildEnvFromShell returns all keys configured to read from shell. +func getAllBuildEnvFromShell(p Provider) []string { + config := p.GetConfig() + if config.BuildEnvFromShell == nil { + return []string{} + } + result := make([]string, len(config.BuildEnvFromShell)) + copy(result, config.BuildEnvFromShell) + return result +} + +// unsetBuildEnvFromShell removes a key from shell environment list. +func unsetBuildEnvFromShell(p Provider, key string) error { + return p.UpdateConfig(func(c *Config) { + if c.BuildEnvFromShell == nil { + return + } + newList := make([]string, 0, len(c.BuildEnvFromShell)) + for _, k := range c.BuildEnvFromShell { + if k != key { + newList = append(newList, k) + } + } + c.BuildEnvFromShell = newList + }) +} diff --git a/pkg/config/buildenv_test.go b/pkg/config/buildenv_test.go index 76a774c68..778f8fc3f 100644 --- a/pkg/config/buildenv_test.go +++ b/pkg/config/buildenv_test.go @@ -1,6 +1,7 @@ package config import ( + "path/filepath" "testing" ) @@ -152,3 +153,358 @@ func TestValidateBuildEnvEntry(t *testing.T) { }) } } + +func TestSetBuildEnvFromSecret(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + key string + secretName string + wantErr bool + }{ + { + name: "valid secret reference", + key: "GITHUB_TOKEN", + secretName: "github-pat", + wantErr: false, + }, + { + name: "invalid key", + key: "invalid-key", + secretName: "some-secret", + wantErr: true, + }, + { + name: "reserved key", + key: "PATH", + secretName: "some-secret", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + configPath := filepath.Join(tempDir, "config.yaml") + provider := NewPathProvider(configPath) + + err := setBuildEnvFromSecret(provider, tt.key, tt.secretName) + if (err != nil) != tt.wantErr { + t.Errorf("setBuildEnvFromSecret(%q, %q) error = %v, wantErr %v", tt.key, tt.secretName, err, tt.wantErr) + } + + if !tt.wantErr { + // Verify it was stored + secretName, exists := getBuildEnvFromSecret(provider, tt.key) + if !exists { + t.Errorf("expected secret reference to be stored") + } + if secretName != tt.secretName { + t.Errorf("expected secret name %q, got %q", tt.secretName, secretName) + } + } + }) + } +} + +func TestSetBuildEnvFromShell(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + key string + wantErr bool + }{ + { + name: "valid shell reference", + key: "ARTIFACTORY_API_KEY", + wantErr: false, + }, + { + name: "invalid key", + key: "invalid-key", + wantErr: true, + }, + { + name: "reserved key", + key: "HOME", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + configPath := filepath.Join(tempDir, "config.yaml") + provider := NewPathProvider(configPath) + + err := setBuildEnvFromShell(provider, tt.key) + if (err != nil) != tt.wantErr { + t.Errorf("setBuildEnvFromShell(%q) error = %v, wantErr %v", tt.key, err, tt.wantErr) + } + + if !tt.wantErr { + // Verify it was stored + exists := getBuildEnvFromShell(provider, tt.key) + if !exists { + t.Errorf("expected shell reference to be stored") + } + } + }) + } +} + +func TestCheckBuildEnvKeyConflict(t *testing.T) { + t.Parallel() + + const githubTokenKey = "GITHUB_TOKEN" + + tempDir := t.TempDir() + configPath := filepath.Join(tempDir, "config.yaml") + provider := NewPathProvider(configPath) + + // Set up a key in each source + key1 := "NPM_CONFIG_REGISTRY" + key2 := githubTokenKey + key3 := "ARTIFACTORY_KEY" + + // Set literal value + err := setBuildEnv(provider, key1, "https://example.com") + if err != nil { + t.Fatalf("failed to set literal value: %v", err) + } + + // Set secret reference + err = setBuildEnvFromSecret(provider, key2, "github-pat") + if err != nil { + t.Fatalf("failed to set secret reference: %v", err) + } + + // Set shell reference + err = setBuildEnvFromShell(provider, key3) + if err != nil { + t.Fatalf("failed to set shell reference: %v", err) + } + + // Test conflicts + tests := []struct { + name string + key string + wantErr bool + }{ + { + name: "conflict with literal value", + key: key1, + wantErr: true, + }, + { + name: "conflict with secret reference", + key: key2, + wantErr: true, + }, + { + name: "conflict with shell reference", + key: key3, + wantErr: true, + }, + { + name: "no conflict", + key: "NEW_VAR", + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := checkBuildEnvKeyConflict(provider, tt.key) + if (err != nil) != tt.wantErr { + t.Errorf("checkBuildEnvKeyConflict(%q) error = %v, wantErr %v", tt.key, err, tt.wantErr) + } + }) + } +} + +func TestGetAllBuildEnvFromSecrets(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + configPath := filepath.Join(tempDir, "config.yaml") + provider := NewPathProvider(configPath) + + // Add some secret references + err := setBuildEnvFromSecret(provider, "GITHUB_TOKEN", "github-pat") + if err != nil { + t.Fatalf("failed to set secret reference: %v", err) + } + + err = setBuildEnvFromSecret(provider, "NPM_TOKEN", "npm-token") + if err != nil { + t.Fatalf("failed to set secret reference: %v", err) + } + + // Get all secrets + secrets := getAllBuildEnvFromSecrets(provider) + if len(secrets) != 2 { + t.Errorf("expected 2 secret references, got %d", len(secrets)) + } + + if secrets["GITHUB_TOKEN"] != "github-pat" { + t.Errorf("expected GITHUB_TOKEN to reference github-pat, got %s", secrets["GITHUB_TOKEN"]) + } + + if secrets["NPM_TOKEN"] != "npm-token" { + t.Errorf("expected NPM_TOKEN to reference npm-token, got %s", secrets["NPM_TOKEN"]) + } + + // Verify it returns a copy (mutation doesn't affect original) + secrets["NEW_KEY"] = "new-secret" + secrets2 := getAllBuildEnvFromSecrets(provider) + if len(secrets2) != 2 { + t.Errorf("expected original to be unchanged, got %d entries", len(secrets2)) + } +} + +func TestGetAllBuildEnvFromShell(t *testing.T) { + t.Parallel() + + const githubTokenKey = "GITHUB_TOKEN" + + tempDir := t.TempDir() + configPath := filepath.Join(tempDir, "config.yaml") + provider := NewPathProvider(configPath) + + // Add some shell references + err := setBuildEnvFromShell(provider, githubTokenKey) + if err != nil { + t.Fatalf("failed to set shell reference: %v", err) + } + + err = setBuildEnvFromShell(provider, "ARTIFACTORY_KEY") + if err != nil { + t.Fatalf("failed to set shell reference: %v", err) + } + + // Get all shell references + shellRefs := getAllBuildEnvFromShell(provider) + if len(shellRefs) != 2 { + t.Errorf("expected 2 shell references, got %d", len(shellRefs)) + } + + // Verify it returns a copy + shellRefs[0] = "MODIFIED" + shellRefs2 := getAllBuildEnvFromShell(provider) + if shellRefs2[0] == "MODIFIED" { + t.Errorf("expected original to be unchanged") + } +} + +func TestUnsetBuildEnvFromSecret(t *testing.T) { + t.Parallel() + + const githubTokenKey = "GITHUB_TOKEN" + + tempDir := t.TempDir() + configPath := filepath.Join(tempDir, "config.yaml") + provider := NewPathProvider(configPath) + + key := githubTokenKey + + // Set and verify + err := setBuildEnvFromSecret(provider, key, "github-pat") + if err != nil { + t.Fatalf("failed to set secret reference: %v", err) + } + + _, exists := getBuildEnvFromSecret(provider, key) + if !exists { + t.Fatalf("expected secret reference to exist") + } + + // Unset and verify + err = unsetBuildEnvFromSecret(provider, key) + if err != nil { + t.Fatalf("failed to unset secret reference: %v", err) + } + + _, exists = getBuildEnvFromSecret(provider, key) + if exists { + t.Errorf("expected secret reference to be removed") + } +} + +func TestUnsetBuildEnvFromShell(t *testing.T) { + t.Parallel() + + const githubTokenKey = "GITHUB_TOKEN" + + tempDir := t.TempDir() + configPath := filepath.Join(tempDir, "config.yaml") + provider := NewPathProvider(configPath) + + key := githubTokenKey + + // Set and verify + err := setBuildEnvFromShell(provider, key) + if err != nil { + t.Fatalf("failed to set shell reference: %v", err) + } + + exists := getBuildEnvFromShell(provider, key) + if !exists { + t.Fatalf("expected shell reference to exist") + } + + // Unset and verify + err = unsetBuildEnvFromShell(provider, key) + if err != nil { + t.Fatalf("failed to unset shell reference: %v", err) + } + + exists = getBuildEnvFromShell(provider, key) + if exists { + t.Errorf("expected shell reference to be removed") + } +} + +func TestSetBuildEnvFromShell_Duplicate(t *testing.T) { + t.Parallel() + + const githubTokenKey = "GITHUB_TOKEN" + + tempDir := t.TempDir() + configPath := filepath.Join(tempDir, "config.yaml") + provider := NewPathProvider(configPath) + + key := githubTokenKey + + // Set once + err := setBuildEnvFromShell(provider, key) + if err != nil { + t.Fatalf("failed to set shell reference: %v", err) + } + + // Set again - should not error, just skip + err = setBuildEnvFromShell(provider, key) + if err != nil { + t.Fatalf("failed to set shell reference again: %v", err) + } + + // Verify it's only in the list once + shellRefs := getAllBuildEnvFromShell(provider) + count := 0 + for _, k := range shellRefs { + if k == key { + count++ + } + } + + if count != 1 { + t.Errorf("expected key to appear once, got %d times", count) + } +} diff --git a/pkg/config/config.go b/pkg/config/config.go index fe1fb645c..747866ccb 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -35,6 +35,8 @@ type Config struct { DefaultGroupMigration bool `yaml:"default_group_migration,omitempty"` DisableUsageMetrics bool `yaml:"disable_usage_metrics,omitempty"` BuildEnv map[string]string `yaml:"build_env,omitempty"` + BuildEnvFromSecrets map[string]string `yaml:"build_env_from_secrets,omitempty"` + BuildEnvFromShell []string `yaml:"build_env_from_shell,omitempty"` } // Secrets contains the settings for secrets management. diff --git a/pkg/config/interface.go b/pkg/config/interface.go index d6d9da30b..ca6b51be1 100644 --- a/pkg/config/interface.go +++ b/pkg/config/interface.go @@ -30,6 +30,18 @@ type Provider interface { GetAllBuildEnv() map[string]string UnsetBuildEnv(key string) error UnsetAllBuildEnv() error + + // Build environment from secrets operations + SetBuildEnvFromSecret(key, secretName string) error + GetBuildEnvFromSecret(key string) (secretName string, exists bool) + GetAllBuildEnvFromSecrets() map[string]string + UnsetBuildEnvFromSecret(key string) error + + // Build environment from shell operations + SetBuildEnvFromShell(key string) error + GetBuildEnvFromShell(key string) (exists bool) + GetAllBuildEnvFromShell() []string + UnsetBuildEnvFromShell(key string) error } // DefaultProvider implements Provider using the default XDG config path @@ -120,6 +132,46 @@ func (d *DefaultProvider) UnsetAllBuildEnv() error { return unsetAllBuildEnv(d) } +// SetBuildEnvFromSecret validates and sets a secret reference for a build environment variable +func (d *DefaultProvider) SetBuildEnvFromSecret(key, secretName string) error { + return setBuildEnvFromSecret(d, key, secretName) +} + +// GetBuildEnvFromSecret retrieves the secret name for a build environment variable +func (d *DefaultProvider) GetBuildEnvFromSecret(key string) (secretName string, exists bool) { + return getBuildEnvFromSecret(d, key) +} + +// GetAllBuildEnvFromSecrets returns all build env secret references +func (d *DefaultProvider) GetAllBuildEnvFromSecrets() map[string]string { + return getAllBuildEnvFromSecrets(d) +} + +// UnsetBuildEnvFromSecret removes a secret reference +func (d *DefaultProvider) UnsetBuildEnvFromSecret(key string) error { + return unsetBuildEnvFromSecret(d, key) +} + +// SetBuildEnvFromShell adds an environment variable name to read from shell at build time +func (d *DefaultProvider) SetBuildEnvFromShell(key string) error { + return setBuildEnvFromShell(d, key) +} + +// GetBuildEnvFromShell checks if a key is configured to read from shell +func (d *DefaultProvider) GetBuildEnvFromShell(key string) bool { + return getBuildEnvFromShell(d, key) +} + +// GetAllBuildEnvFromShell returns all keys configured to read from shell +func (d *DefaultProvider) GetAllBuildEnvFromShell() []string { + return getAllBuildEnvFromShell(d) +} + +// UnsetBuildEnvFromShell removes a key from shell environment list +func (d *DefaultProvider) UnsetBuildEnvFromShell(key string) error { + return unsetBuildEnvFromShell(d, key) +} + // PathProvider implements Provider using a specific config path type PathProvider struct { configPath string @@ -216,6 +268,46 @@ func (p *PathProvider) UnsetAllBuildEnv() error { return unsetAllBuildEnv(p) } +// SetBuildEnvFromSecret validates and sets a secret reference for a build environment variable +func (p *PathProvider) SetBuildEnvFromSecret(key, secretName string) error { + return setBuildEnvFromSecret(p, key, secretName) +} + +// GetBuildEnvFromSecret retrieves the secret name for a build environment variable +func (p *PathProvider) GetBuildEnvFromSecret(key string) (secretName string, exists bool) { + return getBuildEnvFromSecret(p, key) +} + +// GetAllBuildEnvFromSecrets returns all build env secret references +func (p *PathProvider) GetAllBuildEnvFromSecrets() map[string]string { + return getAllBuildEnvFromSecrets(p) +} + +// UnsetBuildEnvFromSecret removes a secret reference +func (p *PathProvider) UnsetBuildEnvFromSecret(key string) error { + return unsetBuildEnvFromSecret(p, key) +} + +// SetBuildEnvFromShell adds an environment variable name to read from shell at build time +func (p *PathProvider) SetBuildEnvFromShell(key string) error { + return setBuildEnvFromShell(p, key) +} + +// GetBuildEnvFromShell checks if a key is configured to read from shell +func (p *PathProvider) GetBuildEnvFromShell(key string) bool { + return getBuildEnvFromShell(p, key) +} + +// GetAllBuildEnvFromShell returns all keys configured to read from shell +func (p *PathProvider) GetAllBuildEnvFromShell() []string { + return getAllBuildEnvFromShell(p) +} + +// UnsetBuildEnvFromShell removes a key from shell environment list +func (p *PathProvider) UnsetBuildEnvFromShell(key string) error { + return unsetBuildEnvFromShell(p, key) +} + // KubernetesProvider is a no-op implementation of Provider for Kubernetes environments. // In Kubernetes, configuration is managed by the cluster, not by local files. type KubernetesProvider struct{} @@ -307,6 +399,46 @@ func (*KubernetesProvider) UnsetAllBuildEnv() error { return nil } +// SetBuildEnvFromSecret is a no-op for Kubernetes environments +func (*KubernetesProvider) SetBuildEnvFromSecret(_, _ string) error { + return nil +} + +// GetBuildEnvFromSecret returns empty for Kubernetes environments +func (*KubernetesProvider) GetBuildEnvFromSecret(_ string) (secretName string, exists bool) { + return "", false +} + +// GetAllBuildEnvFromSecrets returns empty map for Kubernetes environments +func (*KubernetesProvider) GetAllBuildEnvFromSecrets() map[string]string { + return make(map[string]string) +} + +// UnsetBuildEnvFromSecret is a no-op for Kubernetes environments +func (*KubernetesProvider) UnsetBuildEnvFromSecret(_ string) error { + return nil +} + +// SetBuildEnvFromShell is a no-op for Kubernetes environments +func (*KubernetesProvider) SetBuildEnvFromShell(_ string) error { + return nil +} + +// GetBuildEnvFromShell returns false for Kubernetes environments +func (*KubernetesProvider) GetBuildEnvFromShell(_ string) bool { + return false +} + +// GetAllBuildEnvFromShell returns empty slice for Kubernetes environments +func (*KubernetesProvider) GetAllBuildEnvFromShell() []string { + return []string{} +} + +// UnsetBuildEnvFromShell is a no-op for Kubernetes environments +func (*KubernetesProvider) UnsetBuildEnvFromShell(_ string) error { + return nil +} + // NewProvider creates the appropriate config provider based on the runtime environment func NewProvider() Provider { if runtime.IsKubernetesRuntime() { diff --git a/pkg/config/mocks/mock_provider.go b/pkg/config/mocks/mock_provider.go index f59811998..90a31ebb5 100644 --- a/pkg/config/mocks/mock_provider.go +++ b/pkg/config/mocks/mock_provider.go @@ -54,6 +54,34 @@ func (mr *MockProviderMockRecorder) GetAllBuildEnv() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAllBuildEnv", reflect.TypeOf((*MockProvider)(nil).GetAllBuildEnv)) } +// GetAllBuildEnvFromSecrets mocks base method. +func (m *MockProvider) GetAllBuildEnvFromSecrets() map[string]string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetAllBuildEnvFromSecrets") + ret0, _ := ret[0].(map[string]string) + return ret0 +} + +// GetAllBuildEnvFromSecrets indicates an expected call of GetAllBuildEnvFromSecrets. +func (mr *MockProviderMockRecorder) GetAllBuildEnvFromSecrets() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAllBuildEnvFromSecrets", reflect.TypeOf((*MockProvider)(nil).GetAllBuildEnvFromSecrets)) +} + +// GetAllBuildEnvFromShell mocks base method. +func (m *MockProvider) GetAllBuildEnvFromShell() []string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetAllBuildEnvFromShell") + ret0, _ := ret[0].([]string) + return ret0 +} + +// GetAllBuildEnvFromShell indicates an expected call of GetAllBuildEnvFromShell. +func (mr *MockProviderMockRecorder) GetAllBuildEnvFromShell() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAllBuildEnvFromShell", reflect.TypeOf((*MockProvider)(nil).GetAllBuildEnvFromShell)) +} + // GetBuildEnv mocks base method. func (m *MockProvider) GetBuildEnv(key string) (string, bool) { m.ctrl.T.Helper() @@ -69,6 +97,35 @@ func (mr *MockProviderMockRecorder) GetBuildEnv(key any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetBuildEnv", reflect.TypeOf((*MockProvider)(nil).GetBuildEnv), key) } +// GetBuildEnvFromSecret mocks base method. +func (m *MockProvider) GetBuildEnvFromSecret(key string) (string, bool) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetBuildEnvFromSecret", key) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(bool) + return ret0, ret1 +} + +// GetBuildEnvFromSecret indicates an expected call of GetBuildEnvFromSecret. +func (mr *MockProviderMockRecorder) GetBuildEnvFromSecret(key any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetBuildEnvFromSecret", reflect.TypeOf((*MockProvider)(nil).GetBuildEnvFromSecret), key) +} + +// GetBuildEnvFromShell mocks base method. +func (m *MockProvider) GetBuildEnvFromShell(key string) bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetBuildEnvFromShell", key) + ret0, _ := ret[0].(bool) + return ret0 +} + +// GetBuildEnvFromShell indicates an expected call of GetBuildEnvFromShell. +func (mr *MockProviderMockRecorder) GetBuildEnvFromShell(key any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetBuildEnvFromShell", reflect.TypeOf((*MockProvider)(nil).GetBuildEnvFromShell), key) +} + // GetCACert mocks base method. func (m *MockProvider) GetCACert() (string, bool, bool) { m.ctrl.T.Helper() @@ -145,6 +202,34 @@ func (mr *MockProviderMockRecorder) SetBuildEnv(key, value any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetBuildEnv", reflect.TypeOf((*MockProvider)(nil).SetBuildEnv), key, value) } +// SetBuildEnvFromSecret mocks base method. +func (m *MockProvider) SetBuildEnvFromSecret(key, secretName string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SetBuildEnvFromSecret", key, secretName) + ret0, _ := ret[0].(error) + return ret0 +} + +// SetBuildEnvFromSecret indicates an expected call of SetBuildEnvFromSecret. +func (mr *MockProviderMockRecorder) SetBuildEnvFromSecret(key, secretName any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetBuildEnvFromSecret", reflect.TypeOf((*MockProvider)(nil).SetBuildEnvFromSecret), key, secretName) +} + +// SetBuildEnvFromShell mocks base method. +func (m *MockProvider) SetBuildEnvFromShell(key string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SetBuildEnvFromShell", key) + ret0, _ := ret[0].(error) + return ret0 +} + +// SetBuildEnvFromShell indicates an expected call of SetBuildEnvFromShell. +func (mr *MockProviderMockRecorder) SetBuildEnvFromShell(key any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetBuildEnvFromShell", reflect.TypeOf((*MockProvider)(nil).SetBuildEnvFromShell), key) +} + // SetCACert mocks base method. func (m *MockProvider) SetCACert(certPath string) error { m.ctrl.T.Helper() @@ -229,6 +314,34 @@ func (mr *MockProviderMockRecorder) UnsetBuildEnv(key any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UnsetBuildEnv", reflect.TypeOf((*MockProvider)(nil).UnsetBuildEnv), key) } +// UnsetBuildEnvFromSecret mocks base method. +func (m *MockProvider) UnsetBuildEnvFromSecret(key string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UnsetBuildEnvFromSecret", key) + ret0, _ := ret[0].(error) + return ret0 +} + +// UnsetBuildEnvFromSecret indicates an expected call of UnsetBuildEnvFromSecret. +func (mr *MockProviderMockRecorder) UnsetBuildEnvFromSecret(key any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UnsetBuildEnvFromSecret", reflect.TypeOf((*MockProvider)(nil).UnsetBuildEnvFromSecret), key) +} + +// UnsetBuildEnvFromShell mocks base method. +func (m *MockProvider) UnsetBuildEnvFromShell(key string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UnsetBuildEnvFromShell", key) + ret0, _ := ret[0].(error) + return ret0 +} + +// UnsetBuildEnvFromShell indicates an expected call of UnsetBuildEnvFromShell. +func (mr *MockProviderMockRecorder) UnsetBuildEnvFromShell(key any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UnsetBuildEnvFromShell", reflect.TypeOf((*MockProvider)(nil).UnsetBuildEnvFromShell), key) +} + // UnsetCACert mocks base method. func (m *MockProvider) UnsetCACert() error { m.ctrl.T.Helper() diff --git a/pkg/runner/protocol.go b/pkg/runner/protocol.go index e59a2bc0e..3904f631f 100644 --- a/pkg/runner/protocol.go +++ b/pkg/runner/protocol.go @@ -15,6 +15,7 @@ import ( "github.com/stacklok/toolhive/pkg/container/images" "github.com/stacklok/toolhive/pkg/container/templates" "github.com/stacklok/toolhive/pkg/logger" + "github.com/stacklok/toolhive/pkg/secrets" ) // Protocol schemes @@ -125,19 +126,96 @@ func createTemplateData( } // Load build environment variables from configuration - addBuildEnvToTemplate(&templateData) + if err := addBuildEnvToTemplate(&templateData); err != nil { + return templateData, err + } return templateData, nil } // addBuildEnvToTemplate loads build environment variables from config and adds them to template data. -func addBuildEnvToTemplate(templateData *templates.TemplateData) { +// It resolves values from three sources: +// 1. Literal values stored in BuildEnv +// 2. Values from ToolHive secrets (BuildEnvFromSecrets) +// 3. Values from the current shell environment (BuildEnvFromShell) +func addBuildEnvToTemplate(templateData *templates.TemplateData) error { provider := config.NewProvider() - buildEnv := provider.GetAllBuildEnv() - if len(buildEnv) > 0 { - templateData.BuildEnv = buildEnv - logger.Debugf("Loaded %d build environment variable(s) from configuration", len(buildEnv)) + resolvedEnv := make(map[string]string) + + // 1. Add literal values + literalEnv := provider.GetAllBuildEnv() + for k, v := range literalEnv { + resolvedEnv[k] = v + } + + // 2. Resolve values from secrets + secretRefs := provider.GetAllBuildEnvFromSecrets() + if len(secretRefs) > 0 { + secretValues, err := resolveSecretsForBuildEnv(secretRefs) + if err != nil { + return fmt.Errorf("failed to resolve secrets for build env: %w", err) + } + for k, v := range secretValues { + resolvedEnv[k] = v + } + } + + // 3. Resolve values from shell environment + shellRefs := provider.GetAllBuildEnvFromShell() + for _, key := range shellRefs { + value := os.Getenv(key) + if value == "" { + logger.Warnf("Build env variable %s configured to read from shell, but not set in environment", key) + continue + } + resolvedEnv[key] = value + } + + if len(resolvedEnv) > 0 { + templateData.BuildEnv = resolvedEnv + logger.Debugf("Loaded %d build environment variable(s) (redacted for security)", len(resolvedEnv)) + } + + return nil +} + +// resolveSecretsForBuildEnv resolves secret references to their actual values. +func resolveSecretsForBuildEnv(secretRefs map[string]string) (map[string]string, error) { + ctx := context.Background() + configProvider := config.NewProvider() + cfg := configProvider.GetConfig() + + // Check if secrets are set up + if !cfg.Secrets.SetupCompleted { + return nil, secrets.ErrSecretsNotSetup + } + + providerType, err := cfg.Secrets.GetProviderType() + if err != nil { + return nil, fmt.Errorf("failed to get secrets provider type: %w", err) + } + + manager, err := secrets.CreateSecretProvider(providerType) + if err != nil { + return nil, fmt.Errorf("failed to create secrets provider: %w", err) } + + resolved := make(map[string]string, len(secretRefs)) + for key, secretName := range secretRefs { + value, err := manager.GetSecret(ctx, secretName) + if err != nil { + return nil, fmt.Errorf("failed to get secret '%s' for build env variable %s: %w", secretName, key, err) + } + + // Validate the secret value doesn't contain dangerous characters + if err := config.ValidateBuildEnvValue(value); err != nil { + return nil, fmt.Errorf("secret '%s' contains invalid value for build env variable %s: %w", secretName, key, err) + } + + resolved[key] = value + } + + return resolved, nil } // addCACertToTemplate reads and validates a CA certificate, adding it to the template data.