Wire all secret callers to scoped and user providers#4343
Draft
amirejaz wants to merge 3 commits intoscoped-secret-providersfrom
Draft
Wire all secret callers to scoped and user providers#4343amirejaz wants to merge 3 commits intoscoped-secret-providersfrom
amirejaz wants to merge 3 commits intoscoped-secret-providersfrom
Conversation
…elpers 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>
Update every call site that creates a secrets provider to use the appropriate wrapper introduced in Phase 1: - System callers (workload auth tokens, registry credentials, build auth files) now use CreateScopedSecretProvider, placing secrets under the __thv_<scope>_ prefix and hiding them from user-facing commands. - User-facing callers (thv secret commands, REST API, MCP tool server, header secrets, build-env-from-secrets) now use CreateUserSecretProvider, blocking access to __thv_* reserved keys. - RunConfig.WithSecrets and ValidateSecrets now accept separate system and user providers so auth-token resolution and --secret flag resolution use the correct scope independently. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Existing users may have registry tokens and workload auth secrets stored under bare keys (BEARER_TOKEN_, OAUTH_CLIENT_SECRET_, REGISTRY_OAUTH_, etc.) that pre-date the scoped provider wrappers. This migration renames them into the __thv_<scope>_ namespace on first startup so they are accessible via the new scoped providers and hidden from user-facing secret commands. Key design properties: - Write-before-delete ordering: the new key is written before the old is deleted, so a crash mid-migration leaves the secret reachable. - Idempotent: a missing old key is silently skipped, making retries safe. - One-shot: guarded by the SecretScopeMigration config flag; once set, the migration is a cheap config read and returns immediately. - Discovery-based: DiscoverMigrations lists all secrets and matches known system prefixes, so no static registry of workload names is required. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
amirejaz
requested review from
ChrisJBurns,
JAORMX,
blkt,
jhrozek,
lujunsan,
rdimitrov and
yrobla
as code owners
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## scoped-secret-providers #4343 +/- ##
===========================================================
+ Coverage 68.82% 69.31% +0.48%
===========================================================
Files 469 471 +2
Lines 47189 47201 +12
===========================================================
+ Hits 32480 32717 +237
+ Misses 12007 11973 -34
+ Partials 2702 2511 -191 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CreateSecretProvider, giving every caller access to the full flat keyspace. This PR wires each caller to the appropriate wrapper introduced in Phase 1 (Add scoped and user secret providers with system key isolation #4229), so secrets are isolated by design rather than by convention.CreateScopedSecretProvider, writing and reading under the__thv_<scope>_prefix. These keys are invisible to user-facing secret commands.thv secretcommands, REST API, MCP tool server, header secrets, build-env-from-secrets) now useCreateUserSecretProvider, which blocks access to any__thv_*reserved key.RunConfig.WithSecretsandValidateSecretsnow take separatesystemProvideranduserProviderarguments so auth-token resolution and--secretflag resolution use the correct scope independently.This is Phase 3 of the scoped secret store (part of #4192), tracking issue #4227. It must be released together with Phase 4 (#4244 — migration infrastructure).
Type of change
Test plan
go build ./...— clean buildgo test ./pkg/runner/... ./pkg/auth/secrets/... ./pkg/workloads/... ./pkg/mcp/server/...— all passgolangci-lint run— 0 issuesSpecial notes for reviewers
The
WithSecretssignature change (one provider → two providers) is the most structurally significant change. The split is:systemProvider— used forRemoteAuthConfig.ClientSecret,RemoteAuthConfig.BearerToken(both written by the workload auth subsystem underScopeWorkloads)userProvider— used forc.Secrets(--secretflags) andHeaderForward.AddHeadersFromSecret(both user-specified key names)config_buildauthfile.gogets its owngetScopedWorkloadsSecretsManager()helper rather than reusing the user-facinggetSecretsManager(), since build auth files are system-owned (stored bythv config buildauthfile set, not bythv secret set).Generated with Claude Code