stacklok / stacklok/toolhive

User-keyed upstream credential read for delegated agent tokens

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

Nobody has claimed this yet.

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

Description

Blocked on #6403 and #6404 landing — this is the PR that actually consumes both.

What this builds

The user-keyed credential read: a delegated (session-less) token, backed by upstreamCredentialScope: platformUser and carrying sub_ns, reads the same stored upstream credential a browser session already wrote for that user.

Two pieces, and neither is optional — either without the other doesn't produce a well-defined read:

The write-time invariant. One upstream subject per (canonical user, provider). A store call whose UpstreamSubject differs from what's already on record for that user and provider replaces the existing row rather than creating a second one. This is what removes any need for a read-time comparand or disambiguation — there's only ever one row to find.

The read path. A new UserTokenReader capability interface — not a method on the existing TokenReader, a separate interface, implemented by InProcessService, wired as a separate validator field that stays nil unless upstreamCredentialScope: platformUser is set. Before any read:

  • reject a subject bearing the issuer-qualification separator (an externally-issued delegated subject) — never infer this from string shape, check it explicitly
  • reject a synthetic upstream subject
  • both gates run before the storage call, not after
The row-locator interface change

GetLatestUpstreamTokensForUser needs a backend-opaque row handle, the same shape the singleflight fix (#6356) already introduced for ResolveUpstreamTokenRowID. Never a Redis key or a composite string — opaque, resolved by the storage implementation, nothing else assumes its shape. This is a breaking interface change; ship it with a changelog entry, no deprecation cycle.

Refresh failure is per-provider, not all-or-nothing

A stale credential for a provider a given request doesn't need must not fail the whole request. Every consumer of the enriched credential map has to handle a missing provider explicitly, including the point where Cedar denies on absence (it already does — this doesn't change that, just makes "absence" a normal outcome instead of impossible).

Keeping this out of Cedar's claims path

A user-keyed credential is outbound-only. It must never be parsed or substituted as anything Cedar reads — the Agent principal, granted scopes, and resource attributes all come from verified request-token claims and gateway-controlled metadata. Mark Identity with an unexported credential-source flag during enrichment, expose only a narrow predicate to Cedar, and when that predicate is true, Cedar must not read, parse, or log either stored token.

Explicitly not part of this PR
  • Any mode-switch support between session and platformUser
  • Storage migration of any kind
  • The stale-refresh-after-reconnect race and connection-rollback over-deletion questions raised during design — real, but scoped to a later hardening pass once this lands, not blocking it
  • Per-agent credential binding — one shared row per (user, provider) is the whole model here
Vertical coverage required to merge

Per this repo's own rule for this work: an end-to-end kind test is required, not just unit coverage. The decisive assertion is that a delegated token with no session causes the real stored upstream credential to reach a backend's Authorization header — proven by a test backend that actually records and exposes what it received.

Depends on
  • #6403 — upstreamCredentialScope opt-in and issuer trust enforcement
  • #6404 — sub_ns claim
  • #6356 (merged) — the row-locator pattern this reuses
Files
  • pkg/authserver/storage/types.go:582-616UpstreamTokenStorage, the row-locator signature
  • pkg/auth/upstreamtoken/ — new UserTokenReader interface
  • pkg/auth/token.go — validator enrichment, the fallback path when tsid is absent
  • pkg/auth/context.go — credential-source marker on Identity
  • pkg/authz/authorizers/cedar/core.go — the narrow predicate Cedar reads instead

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 row-locator signature in pkg/authserver/storage/types.go:582-616, then read pkg/auth/upstreamtoken/, pkg/auth/token.go, pkg/auth/context.go, and pkg/authz/authorizers/cedar/core.go. Trace the tsid-absent validator path and credential-source handling, then add the required end-to-end kind coverage; done means a delegated token without a session reaches the backend with the stored credential in its Authorization header.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
authentication, backend, security, testing-qa
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.