stacklok / stacklok/toolhive

Add upstreamCredentialScope opt-in and enforce its issuer trust

Open
#6,403 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

authentication bug needs-triage security
Dominant language
Go
Stars
2.2k
Forks
300
Avg merge
1d 15h
Merged PRs (30d)
184

Description

Planned work on non-interactive agent credentials will let a caller read a stored upstream OAuth credential using a durable platform-user identity claim (sub_ns, tracked separately). That claim is trustworthy for credential lookup only when the token carrying it was validated as issued by the embedded ToolHive authserver that owns the credential namespace.

This issue prepares the configuration and trust-enforcement seams for that capability. It does not implement platform-user storage, a functioning UserTokenReader, or agent credential retrieval.

Compatibility contract

Existing deployments must see zero runtime behavior change on upgrade.

Add upstreamCredentialScope to EmbeddedAuthServerConfig (cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go, alongside PrimaryUpstreamProvider and DisableUpstreamTokenInjection):

  • session (permanent default): today's behavior, unchanged. The existing session-based TokenReader remains wired exactly as it is today. UserTokenReader remains nil, so none of the new issuer checks run.
  • platformUser: explicit opt-in to the future durable-user credential model. In this preparatory issue the value is validated and propagated, but runtime activation must fail explicitly as unsupported until the storage-layer issue supplies and wires a functioning UserTokenReader.

Use +kubebuilder:default=session. An object that omits the field, including every object created before this field existed, behaves identically to one that explicitly sets session. The default is permanent; there is no plan to flip it in a later release.

Only a genuinely absent value maps to session. Reject unknown non-empty values; never silently reinterpret a garbage value as the default.

The effective-default rule must also exist in Go configuration handling. Kubernetes defaulting does not cover old serialized RunConfigs, direct vMCP YAML, or direct Go construction.

Scope: trust plumbing only

Introduce a distinct UserTokenReader capability for future durable platform-user lookup, separate from the existing session-based TokenReader.

This issue must:

  1. Add and propagate upstreamCredentialScope through the shared embedded-authserver RunConfig used by the proxy runner and vMCP.
  2. Preserve all existing TokenReader, WithUpstreamTokenReader, MiddlewareRunner.GetUpstreamTokenReader, and session/tsid behavior without signature or semantic changes.
  3. Add the UserTokenReader interface/option seam needed by the later storage work.
  4. Carry the embedded authserver issuer separately on TokenValidatorConfig.
  5. Make NewTokenValidator enforce the strict issuer precondition only when a non-nil UserTokenReader is supplied.
  6. Add scope-gated operator and vMCP configuration validation so the same issuer rule is ready at every entry point.
  7. Reject runtime activation of platformUser as unsupported until the separate storage implementation can construct and wire a real UserTokenReader. Do not silently start in session mode and do not install a no-op reader.

The later storage issue will remove the unsupported activation error when it implements platform-user row semantics and wires the reader.

Trust precondition

When UserTokenReader is non-nil:

  • TokenValidatorConfig.AuthServerIssuer must be non-empty.
  • The incoming OIDC issuer validated by the token validator must exactly equal AuthServerIssuer.
  • Mismatch or absence is a constructor error.
  • There is no fallback to the session reader.

For session scope, UserTokenReader is nil and these checks do not execute.

Example:

scope omitted/session
incoming issuer: https://login.corp.example
embedded AS:     https://auth.example
    -> preserve existing runtime behavior

scope platformUser
incoming issuer: https://login.corp.example
embedded AS:     https://auth.example
    -> reject the new capability configuration

scope platformUser
incoming issuer: https://auth.example
embedded AS:     https://auth.example
    -> trust validation succeeds, then runtime reports that
       platform-user storage is not yet implemented by this issue

Operator validation

Update validateOIDCConfigForEmbeddedAuthServer in cmd/thv-operator/pkg/controllerutil/authserver.go carefully:

  • Preserve all existing session-mode validation behavior.
  • Apply the new issuer-presence and exact-equality checks only when the effective scope is platformUser.
  • Do not gate or otherwise change existing audience/resource checks that already apply to session mode.

The operator should surface invalid platformUser trust configuration before rollout. Session-mode resources, including legacy resources with absent, empty, or mismatched issuers that were previously accepted, must not acquire a new failure solely because of this issue.

vMCP CLI validation

pkg/vmcp/cli/serve.go currently does not call ValidateAuthServerIntegration after side-loading the embedded authserver configuration.

Call it, but gate the newly introduced issuer-presence/equality requirements inside ValidateAuthServerIntegration on effective scope platformUser. Calling the validator must not make existing session-mode vMCP configuration fail after upgrade.

Runtime configuration

Add the scope to the shared authserver.RunConfig, because both proxy runner and vMCP consume that configuration.

Runtime rules:

  • omitted/empty -> effective session;
  • session -> existing runtime behavior;
  • platformUser -> validate trust configuration, then return an explicit unsupported-capability error until the storage implementation lands;
  • any other non-empty value -> configuration error.

An older runtime must not silently ignore an explicitly configured platformUser capability. If current RunConfig version handling cannot guarantee that, add an explicit required-capability/version check rather than relying on unknown-field behavior.

Existing runner behavior

The existing reader in pkg/runner/runner.go is not discarded: it is stored on the runner and obtained by the auth middleware through GetUpstreamTokenReader. Leave that path unchanged.

This issue adds a separate future UserTokenReader seam. It must not convert, wrap, or reinterpret the existing session reader as proof of platform-user trust.

Acceptance tests

Compatibility
  • Omitted scope behaves identically to explicit session.
  • Session mode preserves the existing session TokenReader wiring.
  • Session mode never constructs or attaches UserTokenReader.
  • Session mode does not run the new issuer-presence/equality checks.
  • Existing session-mode operator and vMCP configurations with mismatched issuers retain their previous result.
  • Existing public session-reader APIs and generated mocks remain source-compatible.
Enum/config propagation
  • CRD schema accepts session and platformUser and defaults omission to session.
  • Direct RunConfig handling maps only empty to session.
  • Unknown non-empty values fail validation.
  • CRD-to-RunConfig conversion preserves the selected value for MCPServer and VirtualMCPServer paths.
Trust boundary

Using a mock UserTokenReader supplied directly to NewTokenValidator:

  • matching incoming and authserver issuers succeeds;
  • empty incoming issuer fails;
  • empty authserver issuer fails;
  • mismatched issuers fail;
  • a trailing-slash/string mismatch fails exactly rather than being normalized;
  • no UserTokenReader preserves legacy constructor behavior.
Dormant platform-user activation
  • platformUser with invalid issuer configuration fails with the actionable trust error.
  • platformUser with valid issuer configuration fails explicitly as unsupported until storage support lands.
  • It never falls back to session lookup and never installs a no-op reader.

Explicitly out of scope

  • Platform-user storage keys or authoritative row/pointer design.
  • A functioning UserTokenReader implementation.
  • Construction or runtime wiring of a real UserTokenReader.
  • Reading credentials by sub_ns or canonical user ID.
  • One-row-per-(platformUser, provider) write semantics.
  • Account replacement, refresh compare-and-swap, rollback, logout, and revocation semantics.
  • Storage migration or mode-switch tooling.
  • Per-agent/act.sub credential binding.

Those behaviors belong to the dependent storage and agent-credential issues. This issue only establishes a backward-compatible configuration surface and a fail-closed trust seam that those later changes must use.

Files

Expected areas include:

  • cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go — enum and field.
  • Generated CRDs/deepcopy/operator API documentation.
  • pkg/authserver/config.go — serialized scope, effective default, and validation.
  • cmd/thv-operator/pkg/controllerutil/authserver.go — CRD-to-RunConfig propagation and scope-gated cross-config validation.
  • pkg/auth/token.go — separate UserTokenReader option and conditional issuer validation.
  • pkg/runner/runner.go — preserve existing session reader; reject unsupported platformUser activation until storage wiring exists.
  • pkg/vmcp/config/validator.go and pkg/vmcp/cli/serve.go — scope-gated validation with no session-mode behavior change.
  • Unit and operator integration tests for compatibility, propagation, and dormant activation.

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 pkg/authserver/config.go and cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go to trace scope defaults and propagation, then inspect pkg/auth/token.go and the existing runner reader path. Review the validator entry points in cmd/thv-operator/pkg/controllerutil/authserver.go and pkg/vmcp/cli/serve.go. Done means session behavior remains unchanged, platformUser validates issuer trust and fails explicitly as unsupported, and the listed compatibility and propagation tests pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, kubernetes
Domain
api, authentication, backend, security
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.