Skip to content

Add crash-safe migration for legacy system secret keys#4346

Draft
amirejaz wants to merge 1 commit intophase3-wire-callersfrom
phase4-migration
Draft

Add crash-safe migration for legacy system secret keys#4346
amirejaz wants to merge 1 commit intophase3-wire-callersfrom
phase4-migration

Conversation

@amirejaz
Copy link
Contributor

Summary

  • 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. Without migration, those keys would be unreachable after Phase 3 wires callers to use ScopeWorkloads/ScopeRegistry providers.
  • Adds MigrateSystemKeys and DiscoverMigrations in pkg/secrets/migration.go: discovers bare system keys by known prefix and renames them into the __thv_<scope>_ namespace using write-before-delete ordering for crash safety.
  • Adds CheckAndPerformSecretScopeMigration in pkg/migration/secret_scope.go: guards migration behind a SecretScopeMigration config flag (same pattern as existing migrations) and is triggered from cmd/thv/main.go alongside the other startup migrations.

This is Phase 4 of the scoped secret store (part of #4192), tracking issue #4226. It is stacked on Phase 3 (#4343) and must be released at the same time.

Release note: This PR and the caller-wiring PR (#4343) must be merged before releasing. Neither is a breaking change on its own when no release occurs between the two merges.

Type of change

  • New feature (non-breaking change which adds functionality)

Test plan

  • go build ./... — clean build
  • go test ./pkg/secrets/... ./pkg/migration/... — all pass, including TestMigrateSystemKeys and TestDiscoverMigrations
  • golangci-lint run — 0 issues

Special notes for reviewers

Key design properties of the migration:

  • Write-before-delete: new key is written before the old is deleted, so a crash mid-migration leaves the secret reachable under the new name on retry.
  • Idempotent: if the old key is absent (already migrated or never existed), GetSecret returns a not-found error and the entry is silently skipped.
  • Discovery-based: DiscoverMigrations lists all secrets and matches against SystemKeyPrefixMappings; no static workload registry is required.
  • One-shot: the SecretScopeMigration config flag prevents re-running on every startup after migration completes.
  • Raw provider: migration uses CreateSecretProvider (bare, no wrapper) so it can enumerate and rename both old bare keys and already-scoped __thv_* keys without restriction.

Generated with Claude Code

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>
@github-actions github-actions bot added the size/M Medium PR: 300-599 lines changed label Mar 24, 2026
@amirejaz amirejaz marked this pull request as draft March 24, 2026 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Medium PR: 300-599 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant