diff --git a/docs/Configuring.md b/docs/Configuring.md index 0671954c2e..78c9c09c2e 100644 --- a/docs/Configuring.md +++ b/docs/Configuring.md @@ -22,7 +22,8 @@ The platform leverages [viper](https://github.com/spf13/viper) to help load conf - [Configuration in opentdf-example.yaml](#configuration-in-opentdf-exampleyaml) - [Role Permissions](#role-permissions) - [Managing Authorization Policy](#managing-authorization-policy) - - [Cache Configuration](#cache-configuration) +- [Cache Configuration](#cache-configuration) + - [Secrets In Config](#secrets-in-config) ## Deployment Mode @@ -305,6 +306,63 @@ Root level key `policy` | `list_request_limit_default` | Policy List request limit default when not provided | 1000 | OPENTDF_SERVICES_POLICY_LIST_REQUEST_LIMIT_DEFAULT | | `list_request_limit_max` | Policy List request limit maximum enforced by services | 2500 | OPENTDF_SERVICES_POLICY_LIST_REQUEST_LIMIT_MAX | +## Secrets In Config + +Some service configuration fields are sensitive (secrets). The platform supports convenient, safe ways to provide these values in YAML without leaking them in logs: + +- Literal value: use `literal:` prefix +- Environment variable reference: use `env:` prefix +- File reference (e.g., mounted secret): use `file:` prefix + +Examples: + +```yaml +services: + kas: + # Provide a root key via environment variable + root_key: "env:OPENTDF_SERVICES_KAS_ROOT_KEY" + + # Or as a literal (avoid in production) + # root_key: "literal:493ff7acd07b..." + + # Or from a file path (e.g., k8s secret volume) + # root_key: "file:/var/run/secrets/opentdf/kas_root_key" + + preview: + key_management: true +``` + +Notes: +- Secrets are redacted in logs and JSON output; only a redacted summary is shown. +- For nested secret fields in service-specific configs, prefer the inline `env:` form in YAML to avoid underscore-to-dot mapping issues with environment variables alone. +- You can still use a structured map form for references (YAML): + + Block style (preferred for readability): + ```yaml + services: + kas: + root_key: + fromEnv: OPENTDF_SERVICES_KAS_ROOT_KEY + ``` + + Or referencing a file path: + ```yaml + services: + kas: + root_key: + fromFile: /var/run/secrets/opentdf/kas_root_key + ``` + + Flow style (compact): + ```yaml + services: + kas: + root_key: { fromEnv: OPENTDF_SERVICES_KAS_ROOT_KEY } + # or + root_key: { fromFile: /var/run/secrets/opentdf/kas_root_key } + ``` + + Example: ```yaml diff --git a/service/kas/access/provider.go b/service/kas/access/provider.go index fc6235abb1..8c06e59a88 100644 --- a/service/kas/access/provider.go +++ b/service/kas/access/provider.go @@ -40,7 +40,7 @@ type KASConfig struct { // Deprecated RSACertID string `mapstructure:"rsacertid" json:"rsacertid"` - RootKey string `mapstructure:"root_key" json:"root_key"` + RootKey config.Secret `mapstructure:"root_key" json:"root_key"` KeyCacheExpiration time.Duration `mapstructure:"key_cache_expiration" json:"key_cache_expiration"` diff --git a/service/kas/kas.go b/service/kas/kas.go index 95ffdc3876..886a13c3a9 100644 --- a/service/kas/kas.go +++ b/service/kas/kas.go @@ -2,13 +2,13 @@ package kas import ( "context" + "errors" "fmt" "log/slog" "net" "net/url" "strings" - "github.com/go-viper/mapstructure/v2" kaspb "github.com/opentdf/platform/protocol/go/kas" "github.com/opentdf/platform/protocol/go/kas/kasconnect" "github.com/opentdf/platform/service/internal/security" @@ -21,9 +21,9 @@ import ( ) func OnConfigUpdate(p *access.Provider) serviceregistry.OnConfigUpdateHook { - return func(_ context.Context, cfg config.ServiceConfig) error { + return func(ctx context.Context, cfg config.ServiceConfig) error { var kasCfg access.KASConfig - if err := mapstructure.Decode(cfg, &kasCfg); err != nil { + if err := config.BindServiceConfig(ctx, cfg, &kasCfg); err != nil { return fmt.Errorf("invalid kas cfg [%v] %w", cfg, err) } @@ -46,10 +46,9 @@ func NewRegistration() *serviceregistry.Service[kasconnect.AccessServiceHandler] OnConfigUpdate: onConfigUpdate, RegisterFunc: func(srp serviceregistry.RegistrationParams) (kasconnect.AccessServiceHandler, serviceregistry.HandlerServer) { var kasCfg access.KASConfig - if err := mapstructure.Decode(srp.Config, &kasCfg); err != nil { + if err := config.BindServiceConfig(context.Background(), srp.Config, &kasCfg); err != nil { panic(fmt.Errorf("invalid kas cfg [%v] %w", srp.Config, err)) } - var cacheClient *cache.Cache if kasCfg.KeyCacheExpiration != 0 { var err error @@ -61,54 +60,20 @@ func NewRegistration() *serviceregistry.Service[kasconnect.AccessServiceHandler] } } - var kmgrNames []string - if kasCfg.Preview.KeyManagement { - srp.Logger.Info("preview feature: key management is enabled") - - kasURL, err := determineKASURL(srp, kasCfg) - if err != nil { - panic(fmt.Errorf("failed to determine KAS URL: %w", err)) + if err := handleKeyManagement(srp, kasCfg, p, cacheClient); err != nil { + panic(err) } - - srp.Logger.Debug("determined KAS URL", slog.String("kas_url", kasURL.String())) - - // Configure new delegation service - p.KeyDelegator = trust.NewDelegatingKeyService(NewPlatformKeyIndexer(srp.SDK, kasURL.String(), srp.Logger), srp.Logger, cacheClient) + // Track registered manager names for logging for _, manager := range srp.KeyManagerFactories { p.KeyDelegator.RegisterKeyManager(manager.Name, manager.Factory) - kmgrNames = append(kmgrNames, manager.Name) } - - // Register Basic Key Manager - p.KeyDelegator.RegisterKeyManager(security.BasicManagerName, func(opts *trust.KeyManagerFactoryOptions) (trust.KeyManager, error) { - bm, err := security.NewBasicManager(opts.Logger, opts.Cache, kasCfg.RootKey) - if err != nil { - return nil, err - } - return bm, nil - }) - kmgrNames = append(kmgrNames, security.BasicManagerName) - // Explicitly set the default manager for session key generation. - // This should be configurable, e.g., defaulting to BasicManager or an HSM if available. - p.KeyDelegator.SetDefaultMode(security.BasicManagerName) // Example: default to BasicManager } else { - // Set up both the legacy CryptoProvider and the new SecurityProvider - kasCfg.UpgradeMapToKeyring(srp.OTDF.CryptoProvider) - p.CryptoProvider = srp.OTDF.CryptoProvider - - inProcessService := initSecurityProviderAdapter(p.CryptoProvider, kasCfg, srp.Logger) - - p.KeyDelegator = trust.NewDelegatingKeyService(inProcessService, srp.Logger, nil) - p.KeyDelegator.RegisterKeyManager(inProcessService.Name(), func(*trust.KeyManagerFactoryOptions) (trust.KeyManager, error) { - return inProcessService, nil - }) - // Set default for non-key-management mode - p.KeyDelegator.SetDefaultMode(inProcessService.Name()) - kmgrNames = append(kmgrNames, inProcessService.Name()) + err := handleLegacyMode(srp, kasCfg, p) + if err != nil { + panic(err) + } } - srp.Logger.Info("kas registered trust.KeyManagers", slog.Any("key_managers", kmgrNames)) - p.SDK = srp.SDK p.Logger = srp.Logger p.KASConfig = kasCfg @@ -181,6 +146,70 @@ func determineKASURL(srp serviceregistry.RegistrationParams, kasCfg access.KASCo return kasURL, nil } +func handleKeyManagement(srp serviceregistry.RegistrationParams, kasCfg access.KASConfig, p *access.Provider, cacheClient *cache.Cache) error { + srp.Logger.Info("preview feature: key management is enabled") + srp.Logger.Debug("kas preview settings", slog.Any("preview", kasCfg.Preview)) + + kasURL, err := determineKASURL(srp, kasCfg) + if err != nil { + return fmt.Errorf("failed to determine KAS URL: %w", err) + } + srp.Logger.Debug("determined KAS URL", slog.String("kas_url", kasURL.String())) + + var kmgrNames []string + + // Configure new delegation service + p.KeyDelegator = trust.NewDelegatingKeyService(NewPlatformKeyIndexer(srp.SDK, kasURL.String(), srp.Logger), srp.Logger, cacheClient) + for _, manager := range srp.KeyManagerFactories { + p.KeyDelegator.RegisterKeyManager(manager.Name, manager.Factory) + kmgrNames = append(kmgrNames, manager.Name) + } + kmgrNames = append(kmgrNames, security.BasicManagerName) + + // Register Basic Key Manager + p.KeyDelegator.RegisterKeyManager(security.BasicManagerName, func(opts *trust.KeyManagerFactoryOptions) (trust.KeyManager, error) { + // RootKey is required when key management is enabled. + if kasCfg.RootKey.IsZero() { + return nil, errors.New("root_key is required when preview.key_management is enabled; set OPENTDF_SERVICES_KAS_ROOT_KEY or services.kas.root_key") + } + rk, err := kasCfg.RootKey.Resolve(context.Background()) + if err != nil { + return nil, fmt.Errorf("failed to resolve root_key: %w", err) + } + bm, err := security.NewBasicManager(opts.Logger, opts.Cache, rk) + if err != nil { + return nil, err + } + return bm, nil + }) + + srp.Logger.Info("kas registered trust.KeyManagers", slog.Any("key_managers", kmgrNames)) + + // Explicitly set the default manager for session key generation. + // This should be configurable, e.g., defaulting to BasicManager or an HSM if available. + p.KeyDelegator.SetDefaultMode(security.BasicManagerName) // Example: default to BasicManager + return nil +} + +func handleLegacyMode(srp serviceregistry.RegistrationParams, kasCfg access.KASConfig, p *access.Provider) error { //nolint:unparam // maintains a consistent signature with other handlers + // Set up both the legacy CryptoProvider and the new SecurityProvider + kasCfg.UpgradeMapToKeyring(srp.OTDF.CryptoProvider) + p.CryptoProvider = srp.OTDF.CryptoProvider + + inProcessService := initSecurityProviderAdapter(p.CryptoProvider, kasCfg, srp.Logger) + + p.KeyDelegator = trust.NewDelegatingKeyService(inProcessService, srp.Logger, nil) + p.KeyDelegator.RegisterKeyManager(inProcessService.Name(), func(*trust.KeyManagerFactoryOptions) (trust.KeyManager, error) { + return inProcessService, nil + }) + + srp.Logger.Info("kas registered trust.KeyManagers", slog.Any("key_managers", []string{inProcessService.Name()})) + + // Set default for non-key-management mode + p.KeyDelegator.SetDefaultMode(inProcessService.Name()) + return nil +} + func initSecurityProviderAdapter(cryptoProvider *security.StandardCrypto, kasCfg access.KASConfig, l *logger.Logger) trust.KeyService { var defaults []string var legacies []string diff --git a/service/pkg/config/bind.go b/service/pkg/config/bind.go new file mode 100644 index 0000000000..e6f907b68a --- /dev/null +++ b/service/pkg/config/bind.go @@ -0,0 +1,81 @@ +package config + +import ( + "context" + "errors" + "fmt" + + "github.com/go-playground/validator/v10" + "github.com/go-viper/mapstructure/v2" +) + +// BindOptions controls how service config binding behaves. +type BindOptions struct { + // Eagerly resolve secrets during binding; otherwise they resolve lazily. + EagerResolve bool +} + +// BindOption functional option. +type BindOption func(*BindOptions) + +// WithEagerSecretResolution enables eager secret resolution during bind. +func WithEagerSecretResolution() BindOption { return func(o *BindOptions) { o.EagerResolve = true } } + +// BindServiceConfig decodes a ServiceConfig map into a typed struct target using +// mapstructure with custom decode hooks (e.g., Secret). It optionally validates +// the result and can eagerly resolve secret values. +func BindServiceConfig[T any](ctx context.Context, svcCfg ServiceConfig, out *T, opts ...BindOption) error { + var options BindOptions + for _, o := range opts { + o(&options) + } + + if out == nil { + return errors.New("nil output target") + } + + dec, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ + DecodeHook: mapstructure.ComposeDecodeHookFunc(secretDecodeHook), + Result: out, + TagName: "mapstructure", + Squash: true, + ZeroFields: true, + }) + if err != nil { + return fmt.Errorf("bind decoder: %w", err) + } + if err := dec.Decode(svcCfg); err != nil { + return fmt.Errorf("bind decode: %w", err) + } + + // Eager secret resolution + if options.EagerResolve { + if err := resolveSecrets(ctx, out); err != nil { + return err + } + } + + // Validate struct using go-playground/validator tags, if present + if err := validator.New().Struct(out); err != nil { + var verrs validator.ValidationErrors + if errors.As(err, &verrs) { + return fmt.Errorf("validation failed: %s", verrs.Error()) + } + return fmt.Errorf("validation failed: %w", err) + } + return nil +} + +// resolveSecrets walks the struct and resolves any Secret fields. +func resolveSecrets(ctx context.Context, v any) error { + // Reflectively find Secret fields; keep it minimal and safe. + // We only walk exported fields of structs and slices/maps. + return walk(v, func(s *Secret) error { + // Skip zero-value secrets (unset optional fields) + if s.IsZero() { + return nil + } + _, err := s.Resolve(ctx) + return err + }) +} diff --git a/service/pkg/config/bind_test.go b/service/pkg/config/bind_test.go new file mode 100644 index 0000000000..fdf05506ad --- /dev/null +++ b/service/pkg/config/bind_test.go @@ -0,0 +1,42 @@ +package config + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type demoCfg struct { + User string `mapstructure:"user" validate:"required"` + Pass Secret `mapstructure:"pass"` + Nested struct { + Token Secret `mapstructure:"token"` + } `mapstructure:"nested"` +} + +func TestBindServiceConfig_LiteralAndEnv(t *testing.T) { + t.Setenv("OPENTDF_DEMO_TOKEN", "tok") + + in := ServiceConfig{ + "user": "alice", + "pass": "p@ss", + "nested": map[string]any{ + "token": map[string]any{"fromEnv": "OPENTDF_DEMO_TOKEN"}, + }, + } + + var out demoCfg + err := BindServiceConfig(t.Context(), in, &out, WithEagerSecretResolution()) + require.NoError(t, err) + + assert.Equal(t, "alice", out.User) + + pass, err := out.Pass.Resolve(t.Context()) + require.NoError(t, err) + assert.Equal(t, "p@ss", pass) + + tok, err := out.Nested.Token.Resolve(t.Context()) + require.NoError(t, err) + assert.Equal(t, "tok", tok) +} diff --git a/service/pkg/config/config.go b/service/pkg/config/config.go index 26d1510b85..2795505ddb 100644 --- a/service/pkg/config/config.go +++ b/service/pkg/config/config.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "log/slog" + "strings" "sync" "github.com/go-playground/validator/v10" @@ -190,6 +191,22 @@ func (c *Config) Reload(ctx context.Context) error { assigned = make(map[string]struct{}) orderedViper := viper.NewWithOptions(viper.WithLogger(slog.Default())) + // If an environment loader is present, configure orderedViper to read env directly. + for _, loader := range c.loaders { + if loader.Name() == LoaderNameEnvironmentValue { + // Match the environment loader's behavior + orderedViper.SetEnvPrefix("OPENTDF") // default; overridden below if loader exposes different + if evl, ok := loader.(*EnvironmentValueLoader); ok { + if evl.envPrefix != "" { + orderedViper.SetEnvPrefix(evl.envPrefix) + } + } + orderedViper.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) + orderedViper.AutomaticEnv() + break + } + } + // Loop through loaders in their order of priority. for _, loader := range c.loaders { // The Load call allows the loader to refresh its internal state (e.g., re-read file, re-query DB). @@ -228,6 +245,13 @@ func (c *Config) Reload(ctx context.Context) error { continue } if loaderValue != nil { + // Log loader and key only; never log values in merge path + slog.DebugContext( + ctx, + "config merge", + slog.String("loader", loader.Name()), + slog.String("key", key), + ) orderedViper.Set(key, loaderValue) assigned[key] = struct{}{} } diff --git a/service/pkg/config/decode.go b/service/pkg/config/decode.go new file mode 100644 index 0000000000..0f006b87c3 --- /dev/null +++ b/service/pkg/config/decode.go @@ -0,0 +1,66 @@ +package config + +import ( + "errors" + "fmt" + "reflect" + "strings" +) + +// secretDecodeHook converts supported inputs into Secret. +// Supported inputs: +// - string -> literal secret +// - map[string]any -> reference, e.g., {"fromEnv":"OPENTDF_FOO"} +func secretDecodeHook(from, to reflect.Type, data any) (any, error) { + // Only target Secret type + if to != reflect.TypeOf(Secret{}) { + return data, nil + } + + //nolint:exhaustive // reflect.Kind has many variants; we only handle string and map inputs here + switch from.Kind() { + case reflect.String: + // Support friendly inline directives: "env:VAR", "file:/path", "literal:..." + s := reflect.ValueOf(data).String() + switch { + case strings.HasPrefix(s, "env:"): + v := strings.TrimPrefix(s, "env:") + if v == "" { + return nil, errors.New("empty env directive") + } + return NewEnvSecret(v), nil + case strings.HasPrefix(s, "file:"): + v := strings.TrimPrefix(s, "file:") + if v == "" { + return nil, errors.New("empty file directive") + } + return NewFileSecret(v), nil + case strings.HasPrefix(s, "literal:"): + return NewLiteralSecret(strings.TrimPrefix(s, "literal:")), nil + default: + // Default to literal + return NewLiteralSecret(s), nil + } + case reflect.Map: + // Must be map[string]any (case-insensitive key handling) + m, okm := data.(map[string]any) + if !okm { + return nil, fmt.Errorf("invalid secret map type: %T", data) + } + // Normalize keys to lowercase for robust matching (Viper may lowercase keys) + lower := make(map[string]any, len(m)) + for k, v := range m { + lower[strings.ToLower(k)] = v + } + if env, ok := lower["fromenv"].(string); ok && env != "" { + return NewEnvSecret(env), nil + } + if file, ok2 := lower["fromfile"].(string); ok2 && file != "" { + return NewFileSecret(file), nil + } + // Future: support {"fromURI":"aws-secretsmanager://..."} + return nil, errors.New("unsupported secret map, expected {fromEnv:string} or {fromFile:string}") + default: + return nil, fmt.Errorf("cannot decode %s into Secret", from.Kind()) + } +} diff --git a/service/pkg/config/decode_test.go b/service/pkg/config/decode_test.go new file mode 100644 index 0000000000..422b6d0cf2 --- /dev/null +++ b/service/pkg/config/decode_test.go @@ -0,0 +1,142 @@ +package config + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type inlineCfg struct { + A Secret `mapstructure:"a"` + B Secret `mapstructure:"b"` + C Secret `mapstructure:"c"` +} + +func TestDecodeHook_InlineForms(t *testing.T) { + // Prepare a temp file for file: form + dir := t.TempDir() + f := filepath.Join(dir, "secret.txt") + require.NoError(t, os.WriteFile(f, []byte("from-file"), 0o600)) + t.Setenv("OPENTDF_TEST_INLINE", "from-env") + + in := ServiceConfig{ + "a": "literal:abc", + "b": "env:OPENTDF_TEST_INLINE", + "c": "file:" + f, + } + + var out inlineCfg + require.NoError(t, BindServiceConfig(t.Context(), in, &out)) + + a, err := out.A.Resolve(t.Context()) + require.NoError(t, err) + assert.Equal(t, "abc", a) + b, err := out.B.Resolve(t.Context()) + require.NoError(t, err) + assert.Equal(t, "from-env", b) + c, err := out.C.Resolve(t.Context()) + require.NoError(t, err) + assert.Equal(t, "from-file", c) +} + +func TestDecodeHook_MalformedDirectives(t *testing.T) { + cases := []ServiceConfig{ + {"x": "env:"}, + {"x": "file:"}, + {"x": map[string]any{"fromEnv": ""}}, + {"x": map[string]any{"fromFile": ""}}, + } + for _, in := range cases { + var out struct { + X Secret `mapstructure:"x"` + } + err := BindServiceConfig(t.Context(), in, &out) + require.Error(t, err) + } +} + +// A nested service configuration with multiple tenants and lists of credentials. +type tenantCfg struct { + Credential Secret `mapstructure:"credential"` + Passwords []Secret `mapstructure:"passwords"` +} + +type svcCfgComplex struct { + ClientSecret Secret `mapstructure:"client_secret"` + Tenants map[string]tenantCfg `mapstructure:"tenants"` +} + +func TestBindServiceConfig_NestedTenants_SecretsAndLists(t *testing.T) { + // Prepare env vars and a temp file for fromFile + t.Setenv("OPENTDF_TEST_CLIENT_SECRET", "client-secret") + t.Setenv("OPENTDF_TENANT_A_CRED", "tenant-a-cred") + t.Setenv("OPENTDF_PASS1", "p1") + + dir := t.TempDir() + filePath := filepath.Join(dir, "pass.txt") + require.NoError(t, os.WriteFile(filePath, []byte("from-file\n"), 0o600)) + + in := ServiceConfig{ + "client_secret": "env:OPENTDF_TEST_CLIENT_SECRET", + "tenants": map[string]any{ + "tenantA": map[string]any{ + "credential": "env:OPENTDF_TENANT_A_CRED", + "passwords": []any{ + "env:OPENTDF_PASS1", + "literal:abc", + map[string]any{"fromFile": filePath}, + }, + }, + "tenantB": map[string]any{ + "credential": "literal:credB", + }, + }, + } + + var out svcCfgComplex + // Eagerly resolve to validate that nested secrets are materialized + require.NoError(t, BindServiceConfig(t.Context(), in, &out, WithEagerSecretResolution())) + + // Assert top-level secret + v, err := out.ClientSecret.Resolve(t.Context()) + require.NoError(t, err) + assert.Equal(t, "client-secret", v) + + // Assert tenant map + tenantA, ok := out.Tenants["tenantA"] + require.True(t, ok, "expected tenant 'tenantA' present") + credA, err := tenantA.Credential.Resolve(t.Context()) + require.NoError(t, err) + assert.Equal(t, "tenant-a-cred", credA) + require.Len(t, tenantA.Passwords, 3) + p0, _ := tenantA.Passwords[0].Resolve(t.Context()) + p1, _ := tenantA.Passwords[1].Resolve(t.Context()) + p2, _ := tenantA.Passwords[2].Resolve(t.Context()) + assert.Equal(t, "p1", p0) + assert.Equal(t, "abc", p1) + assert.Equal(t, "from-file", p2) + + // Second tenant literal credential + tenantB, ok := out.Tenants["tenantB"] + require.True(t, ok, "expected tenant 'tenantB' present") + credB, err := tenantB.Credential.Resolve(t.Context()) + require.NoError(t, err) + assert.Equal(t, "credB", credB) +} + +func TestBindServiceConfig_NestedTenants_EagerFailureOnMissingEnv(t *testing.T) { + in := ServiceConfig{ + "tenants": map[string]any{ + "tenantA": map[string]any{ + // Missing env value should cause eager resolution to fail + "credential": "env:OPENTDF_TEST_MISSING_ENV_ABC123", + }, + }, + } + var out svcCfgComplex + err := BindServiceConfig(t.Context(), in, &out, WithEagerSecretResolution()) + require.Error(t, err) +} diff --git a/service/pkg/config/environment_value_loader.go b/service/pkg/config/environment_value_loader.go index ef57c54bf9..d262dab78b 100644 --- a/service/pkg/config/environment_value_loader.go +++ b/service/pkg/config/environment_value_loader.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "log/slog" + "os" "strings" "github.com/spf13/viper" @@ -15,6 +16,7 @@ const LoaderNameEnvironmentValue = "environment-value" type EnvironmentValueLoader struct { allowListMap map[string]struct{} viper *viper.Viper + envPrefix string } // NewEnvironmentValueLoader creates a new Viper-based configuration loader @@ -40,6 +42,7 @@ func NewEnvironmentValueLoader(key string, allowList []string) (*EnvironmentValu result := &EnvironmentValueLoader{ allowListMap: allowListMap, viper: v, + envPrefix: strings.ToUpper(key), } return result, nil } @@ -56,7 +59,41 @@ func (l *EnvironmentValueLoader) Get(key string) (any, error) { // GetConfigKeys returns all the configuration keys found in the environment variables. func (l *EnvironmentValueLoader) GetConfigKeys() ([]string, error) { - return l.viper.AllKeys(), nil + // Start with keys Viper already knows about (from defaults/file loads) + keys := make([]string, 0, len(l.viper.AllKeys())) + keys = append(keys, l.viper.AllKeys()...) + + // Discover environment variables with the configured prefix and convert them to dotted keys + // Example: OPENTDF_SERVICES_KAS_ROOT_KEY -> services.kas.root_key + // Note: Viper treats keys case-insensitively; we normalize to lower-case dotted form here + prefix := l.envPrefix + "_" + for _, env := range os.Environ() { + // env is in the form KEY=VALUE + if !strings.HasPrefix(env, prefix) { + continue + } + // Split only on first '=' to get the key + eq := strings.IndexByte(env, '=') + if eq <= 0 { + continue + } + key := env[:eq] + // Trim prefix + raw := strings.TrimPrefix(key, prefix) + if raw == "" { + continue + } + // Convert to dotted lower-case key path + dotted := strings.ToLower(strings.ReplaceAll(raw, "_", ".")) + // If an allow-list is set, skip keys not present in it + if l.allowListMap != nil { + if _, ok := l.allowListMap[dotted]; !ok { + continue + } + } + keys = append(keys, dotted) + } + return keys, nil } // Load loads the configuration into the provided struct diff --git a/service/pkg/config/secret.go b/service/pkg/config/secret.go new file mode 100644 index 0000000000..e2060da27a --- /dev/null +++ b/service/pkg/config/secret.go @@ -0,0 +1,163 @@ +package config + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io/fs" + "log/slog" + "os" + "strings" + "sync" +) + +// Secret represents a sensitive value. It holds an internal pointer to state +// so copying Secret values does not copy the underlying lock. +type Secret struct{ state *secretState } + +type secretState struct { + mu sync.Mutex + value string + source string + resolved bool +} + +var ( + // ErrSecretNotResolved is returned when attempting to access a secret that hasn't been resolved yet. + ErrSecretNotResolved = errors.New("secret not resolved") + // ErrSecretMissingEnv indicates the requested env var does not exist. + ErrSecretMissingEnv = errors.New("secret env var not set") +) + +const redactedPlaceholder = "[REDACTED]" + +// NewLiteralSecret creates a Secret from a literal value and marks it resolved. +func NewLiteralSecret(v string) Secret { + return Secret{state: &secretState{value: v, source: "literal", resolved: true}} +} + +// NewEnvSecret creates a Secret that will resolve from the given environment variable. +func NewEnvSecret(envName string) Secret { + return Secret{state: &secretState{source: "env:" + envName}} +} + +// NewFileSecret creates a Secret that will resolve from the contents of a file path. +// The value is read as-is; callers can trim/parse if needed. +func NewFileSecret(path string) Secret { + // Normalize to absolute-ish source marker + if !strings.HasPrefix(path, "file:") { + return Secret{state: &secretState{source: "file:" + path}} + } + return Secret{state: &secretState{source: path}} +} + +// Resolve ensures the secret is materialized and returns its value. +// If the secret references an environment variable, it reads it from the process environment. +func (s Secret) Resolve(ctx context.Context) (string, error) { + if s.state == nil { + return "", ErrSecretNotResolved + } + st := s.state + st.mu.Lock() + defer st.mu.Unlock() + + if st.resolved { + return st.value, nil + } + + if st.source == "" { + return "", ErrSecretNotResolved + } + + if ctx != nil { + if err := ctx.Err(); err != nil { + return "", err + } + } + + switch { + case strings.HasPrefix(st.source, "env:"): + envName := strings.TrimPrefix(st.source, "env:") + if envName == "" { + return "", errors.New("empty env directive") + } + if v, ok := os.LookupEnv(envName); ok { + st.value = v + st.resolved = true + return st.value, nil + } + return "", fmt.Errorf("%w: %s", ErrSecretMissingEnv, envName) + case strings.HasPrefix(st.source, "file:"): + path := strings.TrimPrefix(st.source, "file:") + if path == "" { + return "", errors.New("empty file directive") + } + if ctx != nil { + if err := ctx.Err(); err != nil { + return "", err + } + } + b, err := os.ReadFile(path) + if err != nil { + // Mask file not found vs other errors in public message + if errors.Is(err, fs.ErrNotExist) { + return "", fmt.Errorf("secret file not found: %s", path) + } + return "", fmt.Errorf("error reading secret file %s: %w", path, err) + } + st.value = strings.TrimSpace(string(b)) + st.resolved = true + return st.value, nil + case st.source == "literal": + // Should have been resolved already; treat as not resolved if empty. + if st.resolved { + return st.value, nil + } + return "", ErrSecretNotResolved + default: + // Placeholder for future resolvers (e.g., fromURI) + return "", fmt.Errorf("unrecognized secret source: %s", st.source) + } +} + +// String implements fmt.Stringer and returns a redacted representation. +func (s Secret) String() string { return redactedPlaceholder } + +// LogValue implements slog.LogValuer to prevent accidental secret leakage in logs. +func (s Secret) LogValue() slog.Value { + if s.state != nil && s.state.source != "" { + return slog.GroupValue( + slog.String("value", redactedPlaceholder), + slog.String("source", s.state.source), + ) + } + return slog.StringValue(redactedPlaceholder) +} + +// MarshalJSON redacts the value when serialized to JSON. +func (s Secret) MarshalJSON() ([]byte, error) { return json.Marshal(redactedPlaceholder) } + +// Export returns the raw secret value if resolved, otherwise returns an error. +// Intended for explicit, narrow use when the raw value is required. +func (s Secret) Export() (string, error) { + if s.state == nil { + return "", ErrSecretNotResolved + } + s.state.mu.Lock() + defer s.state.mu.Unlock() + if !s.state.resolved { + return "", ErrSecretNotResolved + } + return s.state.value, nil +} + +// IsZero reports whether the secret has no value and no source. +func (s Secret) IsZero() bool { + if s.state == nil { + return true + } + s.state.mu.Lock() + defer s.state.mu.Unlock() + return !s.state.resolved && s.state.source == "" && s.state.value == "" +} diff --git a/service/pkg/config/secret_test.go b/service/pkg/config/secret_test.go new file mode 100644 index 0000000000..c941c5317b --- /dev/null +++ b/service/pkg/config/secret_test.go @@ -0,0 +1,80 @@ +package config + +import ( + "encoding/json" + "os" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSecret_Literal(t *testing.T) { + s := NewLiteralSecret("super-secret") + assert.Equal(t, "[REDACTED]", s.String()) + got, err := s.Resolve(t.Context()) + require.NoError(t, err) + assert.Equal(t, "super-secret", got) + raw, err := s.Export() + require.NoError(t, err) + assert.Equal(t, "super-secret", raw) +} + +func TestSecret_FromEnv(t *testing.T) { + const env = "OPENTDF_TEST_SECRET" + t.Setenv(env, "env-secret") + + s := NewEnvSecret(env) + assert.Equal(t, "[REDACTED]", s.String()) + got, err := s.Resolve(t.Context()) + require.NoError(t, err) + assert.Equal(t, "env-secret", got) +} + +func TestSecret_FromFile_Trim(t *testing.T) { + dir := t.TempDir() + p := dir + "/s.txt" + // Include trailing newline to simulate typical secret mounts + require.NoError(t, os.WriteFile(p, []byte("abc\n"), 0o600)) + s := NewFileSecret(p) + got, err := s.Resolve(t.Context()) + require.NoError(t, err) + assert.Equal(t, "abc", got, "expected trimmed value") +} + +func TestSecret_JSONRedacted(t *testing.T) { + s := NewLiteralSecret("dont-log-me") + b, err := json.Marshal(s) + require.NoError(t, err) + assert.Equal(t, "\"[REDACTED]\"", string(b)) +} + +func TestSecret_Resolve_Concurrent(t *testing.T) { + t.Setenv("OPENTDF_CONCUR", "concur") + s := NewEnvSecret("OPENTDF_CONCUR") + + const n = 50 + var wg sync.WaitGroup + wg.Add(n) + errs := make(chan error, n) + vals := make(chan string, n) + for i := 0; i < n; i++ { + go func() { + defer wg.Done() + v, err := s.Resolve(t.Context()) + if err != nil { + errs <- err + return + } + vals <- v + }() + } + wg.Wait() + close(errs) + close(vals) + require.Empty(t, errs, "expected no resolve errors") + for v := range vals { + assert.Equal(t, "concur", v) + } +} diff --git a/service/pkg/config/walk.go b/service/pkg/config/walk.go new file mode 100644 index 0000000000..ced28e5d18 --- /dev/null +++ b/service/pkg/config/walk.go @@ -0,0 +1,71 @@ +package config + +import ( + "reflect" +) + +// walk traverses v and calls fn for each Secret encountered. +func walk(v any, fn func(*Secret) error) error { + if v == nil { + return nil + } + rv := reflect.ValueOf(v) + return walkValue(rv, fn) +} + +func walkValue(rv reflect.Value, fn func(*Secret) error) error { + if !rv.IsValid() { + return nil + } + // Follow pointers + for rv.Kind() == reflect.Pointer { + if rv.IsNil() { + return nil + } + rv = rv.Elem() + } + + //nolint:exhaustive // We only need to traverse struct, map, slice, and array kinds for secrets + switch rv.Kind() { + case reflect.Struct: + rt := rv.Type() + // Handle Secret itself + if rt == reflect.TypeOf(Secret{}) { + if rv.CanAddr() { + if s, ok := rv.Addr().Interface().(*Secret); ok { + return fn(s) + } + return nil + } + // For non-addressable Secret values (e.g., map index), operate on a copy. + if s, ok := rv.Interface().(Secret); ok { + return fn(&s) + } + return nil + } + // Iterate exported fields + for i := 0; i < rv.NumField(); i++ { + // Only exported fields + if rt.Field(i).IsExported() { + if err := walkValue(rv.Field(i), fn); err != nil { + return err + } + } + } + case reflect.Map: + for _, k := range rv.MapKeys() { + if err := walkValue(rv.MapIndex(k), fn); err != nil { + return err + } + } + case reflect.Slice, reflect.Array: + for i := 0; i < rv.Len(); i++ { + if err := walkValue(rv.Index(i), fn); err != nil { + return err + } + } + default: + // Other kinds: nothing to do + } + return nil +} diff --git a/service/pkg/config/yaml_array_test.go b/service/pkg/config/yaml_array_test.go new file mode 100644 index 0000000000..0a4fb9abe5 --- /dev/null +++ b/service/pkg/config/yaml_array_test.go @@ -0,0 +1,83 @@ +package config + +import ( + "bytes" + "os" + "path/filepath" + "testing" + + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type arrayCfg struct { + Secrets []Secret `mapstructure:"secrets"` +} + +type provider struct { + Name string `mapstructure:"name"` + Password Secret `mapstructure:"password"` +} + +type providersCfg struct { + Providers []provider `mapstructure:"providers"` +} + +func TestBind_FromYAMLArray_SecretsSlice(t *testing.T) { + t.Setenv("OPENTDF_YAML_ARR", "arr-env") + dir := t.TempDir() + fp := filepath.Join(dir, "s.txt") + require.NoError(t, os.WriteFile(fp, []byte("from-file\n"), 0o600)) + + yaml := "" + + "secrets:\n" + + " - \"env:OPENTDF_YAML_ARR\"\n" + + " - { fromFile: \"" + fp + "\" }\n" + + " - \"literal:abc\"\n" + + v := viper.New() + v.SetConfigType("yaml") + require.NoError(t, v.ReadConfig(bytes.NewBufferString(yaml))) + + in := ServiceConfig{ + "secrets": v.Get("secrets"), + } + var out arrayCfg + require.NoError(t, BindServiceConfig(t.Context(), in, &out, WithEagerSecretResolution())) + require.Len(t, out.Secrets, 3) + s0, _ := out.Secrets[0].Resolve(t.Context()) + s1, _ := out.Secrets[1].Resolve(t.Context()) + s2, _ := out.Secrets[2].Resolve(t.Context()) + assert.Equal(t, "arr-env", s0) + assert.Equal(t, "from-file", s1) + assert.Equal(t, "abc", s2) +} + +func TestBind_FromYAMLArray_StructsWithSecretFields(t *testing.T) { + t.Setenv("OPENTDF_PROVIDER_B_PASS", "b-pass") + + yaml := "" + + "providers:\n" + + " - name: a\n" + + " password: \"literal:alpha\"\n" + + " - name: b\n" + + " password: \"env:OPENTDF_PROVIDER_B_PASS\"\n" + + v := viper.New() + v.SetConfigType("yaml") + require.NoError(t, v.ReadConfig(bytes.NewBufferString(yaml))) + + in := ServiceConfig{ + "providers": v.Get("providers"), + } + var out providersCfg + require.NoError(t, BindServiceConfig(t.Context(), in, &out, WithEagerSecretResolution())) + require.Len(t, out.Providers, 2) + assert.Equal(t, "a", out.Providers[0].Name) + assert.Equal(t, "b", out.Providers[1].Name) + p0, _ := out.Providers[0].Password.Resolve(t.Context()) + p1, _ := out.Providers[1].Password.Resolve(t.Context()) + assert.Equal(t, "alpha", p0) + assert.Equal(t, "b-pass", p1) +} diff --git a/service/pkg/server/start_test.go b/service/pkg/server/start_test.go index bff90e197a..6fabf8e639 100644 --- a/service/pkg/server/start_test.go +++ b/service/pkg/server/start_test.go @@ -353,7 +353,7 @@ func (s *StartTestSuite) Test_Start_When_Extra_Service_Registered() { require.NoError(t, err) // Start services with test service - cleanup, err := startServices(context.Background(), startServicesParams{ + cleanup, err := startServices(t.Context(), startServicesParams{ cfg: &config.Config{ Mode: tc.mode, Services: map[string]config.ServiceConfig{