stacklok / stacklok/toolhive

Add scoped secret store to isolate system-managed tokens from user secrets

Open
#4,192 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

authentication enhancement go tech-debt
Dominant language
Go
Stars
2.2k
Forks
300
Avg merge
1d 15h
Merged PRs (30d)
184

Description

Background

ToolHive's secret management uses a flat keyspace — all secrets share the same namespace in the backing store. Users can list, read, and delete any secret via `thv secret list/get/delete`.

Several system-managed tokens are currently stored in this flat namespace and are visible to users:

Key pattern Source
`REGISTRY_OAUTH_` `thv registry login` refresh tokens
`OAUTH_CLIENT_SECRET_` Remote workload OAuth client secrets
`BEARER_TOKEN_` Remote workload bearer tokens
`OAUTH_REFRESH_TOKEN_` Remote workload OAuth refresh tokens

These are internal system credentials. Exposing them via `thv secret list` is confusing and risky — users can accidentally overwrite or delete tokens that workloads depend on.

Proposed Solution

Introduce a `ScopedProvider` and `UserProvider` wrapper layer in `pkg/secrets/`. System callers get a scoped provider that prefixes every key with `_thv_`. User-facing callers get a `UserProvider` wrapper that hides and blocks access to any key starting with `_thv`.

Full design details: THV-0056 RFC

Key prefix format
__thv_registry_REGISTRY_OAUTH_a1b2c3d4      ← registry OAuth refresh tokens
__thv_workloads_BEARER_TOKEN_github-app     ← remote workload auth tokens
__thv_workloads_OAUTH_CLIENT_SECRET_foo     ← remote workload OAuth client secrets
__thv_auth_access_token                     ← reserved for future enterprise login
<user-key>                                  ← user-managed secrets (unchanged)
Scope constants
const (
    SystemKeyPrefix = "__thv_"
    ScopeRegistry   = "registry"   // pkg/registry/auth/
    ScopeWorkloads  = "workloads"  // pkg/auth/secrets/
    ScopeAuth       = "auth"       // future: enterprise login tokens
)
New types in `pkg/secrets/scoped.go`

`ScopedProvider` — for system callers:

  • `GetSecret/SetSecret/DeleteSecret` transparently prefix the key with `_thv_`
  • `ListSecrets` returns only entries in that scope, with prefix stripped
  • `Cleanup` performs a prefix-filtered delete using `BulkDeleteKeys`

`UserProvider` — for user-facing callers:

  • `ListSecrets` filters out any key starting with `_thv`
  • `Get/Set/Delete` of a reserved-prefix key returns `ErrReservedKeyName`
  • `Cleanup` uses `BulkDeleteKeys` to remove only non-system keys
New methods on `EncryptedManager`
// BulkRenameKeys renames multiple keys atomically under a single file lock
// with a single updateFile() call. Store-before-Delete ordering ensures
// concurrent readers always see the value under at least one key.
func (e *EncryptedManager) BulkRenameKeys(renames map[string]string) error

// BulkDeleteKeys deletes multiple keys atomically under a single file lock
// with a single updateFile() call.
func (e *EncryptedManager) BulkDeleteKeys(keys []string) error
Factory helpers in `pkg/secrets/factory.go`
// CreateScopedSecretProvider returns a provider for system-managed tokens.
// Does NOT apply FallbackProvider — system tokens must not come from env vars.
// Returns an error if managerType is EnvironmentType.
func CreateScopedSecretProvider(managerType ProviderType, scope string) (Provider, error)

// CreateUserSecretProvider returns a provider for user-managed secrets.
// Wraps with UserProvider to hide and block system-reserved keys.
func CreateUserSecretProvider(managerType ProviderType) (Provider, error)

Implementation Plan

Phase 1 — Core types (pkg/secrets/scoped.go + scoped_test.go) — additive, no wiring yet ✅ Done

Phase 2 — Factory helpers and bulk EncryptedManager methods

  • Add `BulkRenameKeys` and `BulkDeleteKeys` to `EncryptedManager` in `pkg/secrets/encrypted.go`
  • Add `CreateScopedSecretProvider` and `CreateUserSecretProvider` to `pkg/secrets/factory.go`
  • Update `pkg/secrets/factory_test.go` and `pkg/secrets/encrypted_test.go`

Phase 3 — System callers (ships simultaneously with Phases 4 and 5)

  • `pkg/auth/secrets/secrets.go` → `GetSecretsManager()` uses `ScopeWorkloads`
  • `pkg/registry/factory.go` → `resolveTokenSource()` uses `ScopeRegistry`
  • `cmd/thv/app/registry_login.go:77` → uses `ScopeRegistry`

Phase 4 — User callers (ships simultaneously with Phases 3 and 5)

  • `cmd/thv/app/secret.go`, `pkg/api/v1/secrets.go`
  • `cmd/thv/app/config_buildauthfile.go` (4 calls — `BUILD_AUTH_FILE_*` are user-managed)
  • `cmd/thv/app/config_buildenv.go`, `cmd/thv/app/header_flags.go`
  • `pkg/runner/protocol.go`, `pkg/runner/runner.go`, `pkg/workloads/manager.go`
  • `pkg/mcp/server/list_secrets.go`, `set_secret.go`

Phase 5 — Key migration (ships simultaneously with Phases 3 and 4)

  • Add `pkg/secrets/migration.go` with `MigrateSystemKeys`
  • Uses `BulkRenameKeys` for a single atomic file write — not optional
  • Guard with config flag to run exactly once

Phase 6 — Admin escape hatch

  • Add `--system` flag to `thv secret list` and `thv secret delete`
  • Document as advanced/emergency commands for managing stale or leaked system tokens

Risks

Risk Mitigation
Existing users lose cached tokens Phase 5 migration ships simultaneously with Phases 3+4
FallbackProvider + ScopedProvider ordering CreateScopedSecretProvider skips fallback entirely
UserProvider.Cleanup() wipes system tokens BulkDeleteKeys scoped to non-system keys only
EnvironmentType provider used as system store CreateScopedSecretProvider returns error for EnvironmentType
1Password / path separator conflict __thv_ prefix uses no slashes — conflict eliminated by construction

Acceptance Criteria

  • ScopedProvider and UserProvider implemented in pkg/secrets/scoped.go with unit tests
  • BulkRenameKeys and BulkDeleteKeys added to EncryptedManager
  • CreateScopedSecretProvider and CreateUserSecretProvider factory helpers added
  • thv secret list no longer shows REGISTRY_OAUTH_*, OAUTH_CLIENT_SECRET_*, BEARER_TOKEN_*, or OAUTH_REFRESH_TOKEN_* keys
  • thv secret get/set/delete with an __thv_ prefix returns a clear error message
  • All system callers updated to use CreateScopedSecretProvider (including registry_login.go)
  • All user callers updated to use CreateUserSecretProvider
  • Key migration runs on first startup and atomically moves legacy system keys to scoped equivalents
  • thv secret list --system and thv secret delete --system available for emergency use
  • All existing secret management behavior is unchanged for user-managed secrets
  • ScopeAuth constant reserved for future enterprise login tokens

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the THV-0056 RFC, then inspect pkg/secrets/scoped.go and scoped_test.go to understand the completed core types. Next read pkg/secrets/encrypted.go, pkg/secrets/factory.go, and their tests before tracing the listed system and user callers. Run the relevant secrets tests; done means atomic migration, scoped system access, protected user access, updated callers, and the documented CLI escape hatch.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend, cli, security
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.