Skip to content

Preserve legacy encrypted stores until users opt in to Argon2id migration #6710

Description

@jhrozek

Problem

ToolHive v0.50 introduced an on-disk format change for the encrypted secrets provider in #6657. It replaces the legacy raw AES-GCM ciphertext, whose key was derived with SHA-256(password), with a framed THVSEC v1 format using a per-file salt and Argon2id.

The security improvement is desirable, but the current migration behavior is unsafe for ordinary local upgrades:

  1. A v0.50+ binary opens an existing legacy secrets store.

  2. NewEncryptedManager transparently rewrites it to the new format during provider construction, including for read-only operations.

  3. Any already-running or independently installed pre-v0.50 binary that shares the same user/XDG store can no longer read the file.

  4. The old process commonly fails later, when it next reads or persists OAuth state, with an AES-GCM error such as:

    failed to persist initial refresh token
    unable to decrypt secrets file: cipher: message authentication failed
    

This presents as an OAuth/keyring/corruption problem rather than an upgrade compatibility boundary.

Plausible reproduction

A mixed-version environment does not require a user to deliberately install two versions.

# Run with a pre-v0.50 binary.
thv llm proxy start &

# Upgrade the thv executable on disk to v0.50+.

# Run a new thv command that opens the same encrypted store.
thv llm token

The LLM proxy continues to execute the old binary after the executable on disk is replaced. The new command migrates the shared store. Once the old proxy needs to read or persist a refresh token, it cannot decrypt the THVSEC file.

Other affected topologies include:

  • thv serve still running while a newer CLI or Studio binary opens the store;
  • detached workloads launched before the upgrade;
  • ToolHive Desktop's bundled CLI/server and a separately installed PATH CLI;
  • independently installed or pinned CLI binaries using the same user-level XDG state.

See:

  • pkg/secrets/encrypted.go
  • pkg/auth/tokensource/tokensource.go
  • cmd/thv/app/llm.go
  • pkg/workloads/manager.go
  • docs/arch/04-secrets-management.md

Why this matters

The current behavior makes a routine upgrade perform an irreversible compatibility cutover as a side effect of opening the store. Users cannot reasonably inventory every locally running ToolHive process or every independently installed binary before running a new command.

The current documentation advises stopping old processes before the first access, but that is an operational workaround rather than a safe default for desktop, CLI, background-server, and detached-workload deployments.

Proposed behavior

Separate compatibility from the optional security upgrade:

Existing store state Normal behavior
No store / empty store Create a THVSEC / Argon2id store
Legacy store Read and continue writing the legacy format
THVSEC v1 store Read and continue writing v1
Unsupported future format Fail without modifying the store

ToolHive must not automatically convert an existing legacy store during provider construction or any ordinary secret mutation.

Instead, offer a user-initiated protection upgrade, for example:

thv secret upgrade-protection

The command/UI should clearly state:

This upgrades the local encrypted secrets store to Argon2id protection. ToolHive versions before v0.50 can no longer use this store. Stop or upgrade any other local ToolHive installations, servers, proxies, and workloads before continuing.

The user, who knows their own local installation topology, confirms the cutover. Automation can use an explicit non-interactive confirmation flag.

No process scanning or automatic detection is required for the initial design: it is platform-specific, incomplete, and risks providing false assurance.

Implementation notes

  • Removing the constructor migration alone is insufficient. writeFileSecrets currently always emits the framed format, so an ordinary secret update or refresh-token rotation would still silently convert a legacy store.
  • The encrypted manager should retain the format it read and preserve that format for ordinary writes.
  • Only the explicit protection-upgrade flow may convert legacy -> THVSEC.
  • That flow should use the existing file lock, reread/authenticate the legacy file under the lock, atomically replace it with the framed representation, and verify it can reopen the result.
  • Do not introduce a default dual-file/dual-write bridge. It retains the weak representation, makes two copies of every secret authoritative, and creates crash-consistency and token-rotation reconciliation problems.

Recovery for stores already migrated by v0.50

This change cannot make pre-v0.50 binaries understand an already-migrated THVSEC store. The supported recovery remains:

  1. Preserve the current secrets file and keyring password.
  2. Upgrade every local ToolHive executable that accesses the store to v0.50+.
  3. Restart long-running servers, proxies, and detached workloads.

Do not recommend deleting the secrets store or resetting the keyring for this format mismatch.

Acceptance criteria

  • Opening a legacy encrypted store never rewrites it.
  • Normal set/delete/cleanup/token-persistence operations preserve legacy encoding when the source store is legacy.
  • New empty stores use THVSEC / Argon2id.
  • Existing THVSEC stores remain readable and writable.
  • A user-initiated protection upgrade converts a legacy store atomically and verifies the converted store before success.
  • The opt-in flow warns that older ToolHive binaries become incompatible and requires confirmation.
  • Errors distinguish unsupported/incompatible store formats from a wrong password or ciphertext corruption where possible.
  • Documentation describes normal compatibility behavior, opt-in cutover, and recovery for users already converted by v0.50.
  • Cross-version tests cover an old LLM proxy or detached workload sharing a legacy store with a new CLI, and prove that normal new-version operations do not break the old process.
  • Tests cover failed explicit migration and prove the original legacy store remains usable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs-triageIssue needs initial triage by a maintainer

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions