Skip to content

Commit 1c276b4

Browse files
amirejazclaude
andauthored
Add factory helpers for user and scoped secret providers (#4242)
* Add CreateUserSecretProvider and CreateScopedSecretProvider factory helpers Expose convenience constructors that wrap the base secret provider in either a UserProvider (blocks system-reserved __thv_ keys for user-facing callers) or a ScopedProvider (namespaces all operations under a given scope for internal callers such as the registry or workloads subsystems). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Use t.Context() instead of context.Background() in factory tests Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Update CreateScopedSecretProvider to use SecretScope type Follow-up from the SecretScope type change: update the factory helper signature to accept SecretScope instead of string. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Replace separate factory helpers with functional options on CreateProvider Replace CreateUserSecretProvider and CreateScopedSecretProvider with a single CreateProvider function that accepts ProviderOptions, consistent with the functional options pattern already used in config_builder.go. - Add ProviderOption type with WithUserFacing() and WithScope(scope) - Validate mutual exclusivity: combining both options returns an error - Without options, CreateProvider behaves like CreateSecretProvider - Update tests to cover the new API and the mutual exclusion invariant Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent c63b9eb commit 1c276b4

2 files changed

Lines changed: 147 additions & 2 deletions

File tree

pkg/secrets/factory.go

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
1+
// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc.
22
// SPDX-License-Identifier: Apache-2.0
33

44
package secrets
@@ -271,6 +271,69 @@ func CreateSecretProviderWithPassword(managerType ProviderType, password string)
271271
return primary, nil
272272
}
273273

274+
// ProviderOption configures how CreateProvider wraps the underlying provider.
275+
type ProviderOption func(*providerOptions) error
276+
277+
// providerOptions holds the accumulated state of all ProviderOptions passed to CreateProvider.
278+
type providerOptions struct {
279+
scope *SecretScope // nil = no scoping
280+
userFacing bool
281+
}
282+
283+
// WithScope returns a ProviderOption that wraps the provider with a ScopedProvider
284+
// for the given scope, keeping system secrets isolated under "__thv_<scope>_".
285+
// Mutually exclusive with WithUserFacing.
286+
func WithScope(scope SecretScope) ProviderOption {
287+
return func(o *providerOptions) error {
288+
if o.userFacing {
289+
return errors.New("WithScope and WithUserFacing are mutually exclusive")
290+
}
291+
o.scope = &scope
292+
return nil
293+
}
294+
}
295+
296+
// WithUserFacing returns a ProviderOption that wraps the provider with a UserProvider,
297+
// blocking access to any key that starts with the system prefix "__thv_".
298+
// Suitable for CLI, API, and MCP tool server callers.
299+
// Mutually exclusive with WithScope.
300+
func WithUserFacing() ProviderOption {
301+
return func(o *providerOptions) error {
302+
if o.scope != nil {
303+
return errors.New("WithUserFacing and WithScope are mutually exclusive")
304+
}
305+
o.userFacing = true
306+
return nil
307+
}
308+
}
309+
310+
// CreateProvider creates a secret Provider for the given manager type, optionally
311+
// wrapped according to the supplied options.
312+
//
313+
// Without options it behaves identically to CreateSecretProvider.
314+
// Pass WithUserFacing() for CLI/API callers or WithScope(scope) for internal callers.
315+
func CreateProvider(managerType ProviderType, opts ...ProviderOption) (Provider, error) {
316+
inner, err := CreateSecretProvider(managerType)
317+
if err != nil {
318+
return nil, err
319+
}
320+
321+
options := &providerOptions{}
322+
for _, opt := range opts {
323+
if err := opt(options); err != nil {
324+
return nil, err
325+
}
326+
}
327+
328+
if options.userFacing {
329+
return NewUserProvider(inner), nil
330+
}
331+
if options.scope != nil {
332+
return NewScopedProvider(inner, *options.scope), nil
333+
}
334+
return inner, nil
335+
}
336+
274337
// shouldEnableFallback determines if environment variable fallback should be enabled
275338
func shouldEnableFallback() bool {
276339
// Check for explicit opt-out

pkg/secrets/factory_test.go

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
1+
// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc.
22
// SPDX-License-Identifier: Apache-2.0
33

44
package secrets_test
@@ -112,3 +112,85 @@ func TestEnvVarPrefix(t *testing.T) { //nolint:paralleltest
112112
assert.Equal(t, "TOOLHIVE_SECRET_", secrets.EnvVarPrefix)
113113
})
114114
}
115+
116+
func TestCreateProvider_WithUserFacing(t *testing.T) { //nolint:paralleltest
117+
t.Run("environment provider returns user provider", func(t *testing.T) { //nolint:paralleltest
118+
provider, err := secrets.CreateProvider(secrets.EnvironmentType, secrets.WithUserFacing())
119+
require.NoError(t, err)
120+
require.NotNil(t, provider)
121+
122+
// UserProvider inherits read-only capabilities from the environment provider
123+
caps := provider.Capabilities()
124+
assert.True(t, caps.CanRead)
125+
assert.False(t, caps.CanWrite)
126+
})
127+
128+
t.Run("blocks system-reserved keys", func(t *testing.T) { //nolint:paralleltest
129+
provider, err := secrets.CreateProvider(secrets.EnvironmentType, secrets.WithUserFacing())
130+
require.NoError(t, err)
131+
132+
_, err = provider.GetSecret(t.Context(), "__thv_registry_foo")
133+
require.ErrorIs(t, err, secrets.ErrReservedKeyName)
134+
})
135+
136+
t.Run("allows non-system keys", func(t *testing.T) { //nolint:paralleltest
137+
provider, err := secrets.CreateProvider(secrets.EnvironmentType, secrets.WithUserFacing())
138+
require.NoError(t, err)
139+
140+
// A regular key should not be blocked (may return not-found, but not ErrReservedKeyName)
141+
_, err = provider.GetSecret(t.Context(), "my-api-key")
142+
require.NotErrorIs(t, err, secrets.ErrReservedKeyName)
143+
})
144+
145+
t.Run("unknown provider returns error", func(t *testing.T) { //nolint:paralleltest
146+
provider, err := secrets.CreateProvider(secrets.ProviderType("unknown"), secrets.WithUserFacing())
147+
assert.Error(t, err)
148+
assert.Nil(t, provider)
149+
})
150+
}
151+
152+
func TestCreateProvider_WithScope(t *testing.T) { //nolint:paralleltest
153+
t.Run("environment provider returns scoped provider", func(t *testing.T) { //nolint:paralleltest
154+
provider, err := secrets.CreateProvider(secrets.EnvironmentType, secrets.WithScope(secrets.ScopeRegistry))
155+
require.NoError(t, err)
156+
require.NotNil(t, provider)
157+
158+
// ScopedProvider inherits read-only capabilities from the environment provider
159+
caps := provider.Capabilities()
160+
assert.True(t, caps.CanRead)
161+
assert.False(t, caps.CanWrite)
162+
})
163+
164+
t.Run("scopes key access to given scope", func(t *testing.T) { //nolint:paralleltest
165+
provider, err := secrets.CreateProvider(secrets.EnvironmentType, secrets.WithScope(secrets.ScopeRegistry))
166+
require.NoError(t, err)
167+
168+
// Any get on an environment provider will return not-found; the key must not be blocked
169+
_, err = provider.GetSecret(t.Context(), "my-token")
170+
require.NotErrorIs(t, err, secrets.ErrReservedKeyName)
171+
})
172+
173+
t.Run("unknown provider returns error", func(t *testing.T) { //nolint:paralleltest
174+
provider, err := secrets.CreateProvider(secrets.ProviderType("unknown"), secrets.WithScope(secrets.ScopeRegistry))
175+
assert.Error(t, err)
176+
assert.Nil(t, provider)
177+
})
178+
}
179+
180+
func TestCreateProvider_MutualExclusion(t *testing.T) { //nolint:paralleltest
181+
t.Run("WithScope and WithUserFacing are mutually exclusive", func(t *testing.T) { //nolint:paralleltest
182+
_, err := secrets.CreateProvider(secrets.EnvironmentType,
183+
secrets.WithScope(secrets.ScopeRegistry),
184+
secrets.WithUserFacing(),
185+
)
186+
require.Error(t, err)
187+
})
188+
189+
t.Run("WithUserFacing and WithScope are mutually exclusive", func(t *testing.T) { //nolint:paralleltest
190+
_, err := secrets.CreateProvider(secrets.EnvironmentType,
191+
secrets.WithUserFacing(),
192+
secrets.WithScope(secrets.ScopeRegistry),
193+
)
194+
require.Error(t, err)
195+
})
196+
}

0 commit comments

Comments
 (0)