From 98b97718b693a3ef105e3c44e4892119286e07c2 Mon Sep 17 00:00:00 2001 From: MK Date: Fri, 17 Jul 2026 09:23:45 -0400 Subject: [PATCH 1/2] feat(mcp): delegated per-user token resolution for type=user (#317) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #326 landed the type=user resolver as a pure stub — every call returned ErrNoToken. This makes it actually resolve a per-REQUESTING-USER token from the platform token endpoint (§19 delegated contract), the token-acquisition core of the delegated path. - `delegatedTokenSource`: POSTs `{server, subject}` to the platform token endpoint and caches the access token PER SUBJECT, so distinct users never share a token. The lock is not held across the network fetch, so a slow fetch for user A never blocks user B (the multi-user path is the point of #317). A platform 401/403/404 → ErrNoToken (auth-required), so a user without a grant stays lazy/non-blocking. Refresh token never reaches the agent (invariant 8). - `buildAuthFn` type=user: reads the requesting user from `auth.IdentityFromContext(ctx)` (email preferred, then user id) and resolves that subject's token. authFn runs per-request with the request ctx, so a shared connection carries per-user tokens on the call path. No user in ctx → ErrNoToken (lazy; never at startup). - `NewServer` type=user now requires the top-level platform block (it resolves via the platform endpoint), same as type=platform. - Extracted `doPlatformTokenRequest` shared by the agent-principal and delegated sources; the agent-principal path (#326) is behavior-preserved. Docs: configuration.md gains a "Managed identity (platform / user)" section — #326 shipped both types without config docs. Scope: this is the token-acquisition layer. The per-user CONNECTION lifecycle (lazy establishment so `initialize` runs under a user's session) and carrying the raw OIDC assertion for a broker-side ID-JAG exchange are the follow-ons; the platform's vaulted-3LO path resolves on subject alone today. Tests: per-subject fetch+cache (distinct users → distinct tokens, same user cached), platform-403 → ErrNoToken, buildAuthFn resolves subject from ctx / lazy without a user. Updated #326's rules test for the new platform-block requirement on type=user. --- docs/mcp/configuration.md | 45 ++++++++ forge-core/mcp/platform_delegated_test.go | 119 +++++++++++++++++++++ forge-core/mcp/platform_token.go | 123 +++++++++++++++++++--- forge-core/mcp/platform_token_test.go | 14 ++- forge-core/mcp/server.go | 48 ++++++++- 5 files changed, 323 insertions(+), 26 deletions(-) create mode 100644 forge-core/mcp/platform_delegated_test.go diff --git a/docs/mcp/configuration.md b/docs/mcp/configuration.md index 5862732..db5cb61 100644 --- a/docs/mcp/configuration.md +++ b/docs/mcp/configuration.md @@ -67,10 +67,55 @@ MCPs). | `oauth` | *(none required — see Discovery)* | Hosted MCPs (Linear, Notion, GitHub hosted) | | `bearer` | `token_env` | In-cluster sidecars; CI machine-to-machine | | `static` | `token_env` | Same as bearer; named for clarity | +| `platform` | top-level `platform` block | **Managed:** agent-principal (service) identity resolved by the platform | +| `user` | top-level `platform` block | **Managed:** delegated per-requesting-user identity, resolved by the platform | `token_env` is the name of an environment variable; the variable's value is read at runtime — never stored in `forge.yaml`. +#### Managed identity — `type: platform` / `type: user` + +In a platform-managed deployment, Forge doesn't hold long-lived +credentials at all: it fetches a **short-lived access token** from the +platform's token endpoint per request. The refresh token stays platform-side +and never reaches the agent. Both types use the top-level `platform` block: + +```yaml +platform: + token_endpoint: ${INITIALIZ_TOKEN_ENDPOINT} # ${VAR}-expanded at use + agent_identity: ${FORGE_PLATFORM_TOKEN} # the agent's platform credential + +mcp: + servers: + - name: atlassian-read + url: https://mcp.atlassian.com/mcp + auth: { type: platform, ref: mcp.atlassian } # agent-principal + required: true # startup-viable: no human needed + - name: atlassian-write + url: https://mcp.atlassian.com/mcp + auth: { type: user, ref: mcp.atlassian } # delegated per-user + required: false # lazy: no user at startup +``` + +- **`type: platform`** — agent-principal. Forge POSTs `{server: }` with + the agent credential; startup-viable (no user, no login). `required: true` + is valid. +- **`type: user`** (#317) — delegated. Forge resolves the **requesting user's** + identity from the authenticated request and POSTs `{server: , subject}`, + so each user gets **their own** token (cached per subject). It is **inherently + lazy**: `required: true` is rejected (there is no user at startup), and until + the platform has a grant for that user the call fails auth-required — the + server simply carries no tools for a user who hasn't consented yet. +- **`ref`** names the platform tool-registry entry the token is authorized + against (defaults to the server name). +- **Egress:** the platform token-endpoint host is auto-merged into the + allowlist. + +> The platform materializes both the `platform` block and the per-identity +> server entries (same URL, split by `type`). For the **standalone** (no +> platform) equivalents, use `type: oauth` — 3-legged `forge mcp login` for a +> user, or `grant: client_credentials` for an agent-principal (above). + #### OAuth discovery & dynamic client registration (#316) For `type: oauth`, `client_id` / `authorize_url` / `token_url` are all diff --git a/forge-core/mcp/platform_delegated_test.go b/forge-core/mcp/platform_delegated_test.go new file mode 100644 index 0000000..88d94aa --- /dev/null +++ b/forge-core/mcp/platform_delegated_test.go @@ -0,0 +1,119 @@ +package mcp + +import ( + "context" + "encoding/json" + "errors" + "io" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + + "github.com/initializ/forge/forge-core/auth" + "github.com/initializ/forge/forge-core/types" +) + +// delegatedServer is a fake platform token endpoint for the type=user +// path (#317): it echoes the requested subject into the token, counts +// calls, and can 403 a named subject to simulate "no grant yet". +func delegatedServer(t *testing.T, calls *atomic.Int32, denySubject string) *httptest.Server { + t.Helper() + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if calls != nil { + calls.Add(1) + } + body, _ := io.ReadAll(r.Body) + var in struct{ Server, Subject string } + _ = json.Unmarshal(body, &in) + if in.Subject == "" { + t.Errorf("delegated request missing subject: %s", body) + } + if in.Subject == denySubject { + w.WriteHeader(http.StatusForbidden) + return + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "access_token": "tok-for-" + in.Subject, + "expires_in": 3600, + }) + })) +} + +// TestDelegatedTokenSource_PerSubjectFetchAndCache: distinct users get +// distinct tokens (subject sent in the body), and the same user's token is +// cached — one fetch per subject. +func TestDelegatedTokenSource_PerSubjectFetchAndCache(t *testing.T) { + var calls atomic.Int32 + srv := delegatedServer(t, &calls, "") + defer srv.Close() + + d := newDelegatedTokenSource(PlatformSourceConfig{ + TokenEndpoint: srv.URL, AgentIdentity: "agent-cred", Ref: "atlassian", HTTPClient: srv.Client(), + }) + ctx := context.Background() + + a, err := d.TokenForSubject(ctx, "alice@corp.com") + if err != nil || a != "tok-for-alice@corp.com" { + t.Fatalf("alice: %q %v", a, err) + } + b, err := d.TokenForSubject(ctx, "bob@corp.com") + if err != nil || b != "tok-for-bob@corp.com" { + t.Fatalf("bob: %q %v", b, err) + } + if a == b { + t.Fatal("distinct users must get distinct tokens") + } + // alice again — served from the per-subject cache, no new fetch. + if _, err := d.TokenForSubject(ctx, "alice@corp.com"); err != nil { + t.Fatalf("alice cached: %v", err) + } + if n := calls.Load(); n != 2 { + t.Errorf("platform hit %d times, want 2 (one per distinct subject; alice cached)", n) + } +} + +// TestDelegatedTokenSource_NoGrantIsAuthRequired: a platform 403 for a +// subject (no grant yet) surfaces as ErrNoToken — auth-required, lazy — +// not a hard error. +func TestDelegatedTokenSource_NoGrantIsAuthRequired(t *testing.T) { + srv := delegatedServer(t, nil, "carol@corp.com") + defer srv.Close() + d := newDelegatedTokenSource(PlatformSourceConfig{ + TokenEndpoint: srv.URL, AgentIdentity: "c", Ref: "r", HTTPClient: srv.Client(), + }) + if _, err := d.TokenForSubject(context.Background(), "carol@corp.com"); !errors.Is(err, ErrNoToken) { + t.Fatalf("no grant must be ErrNoToken (auth-required), got: %v", err) + } +} + +// TestBuildAuthFn_User_ResolvesSubjectFromContext: the type=user authFn +// pulls the requesting user from the request ctx and resolves that user's +// token; with no user in ctx it fails lazily with ErrNoToken. +func TestBuildAuthFn_User_ResolvesSubjectFromContext(t *testing.T) { + srv := delegatedServer(t, nil, "") + defer srv.Close() + + spec := types.MCPServer{ + Name: "atlassian-read", URL: "https://mcp.atlassian.com/mcp", + Auth: &types.MCPAuth{Type: "user", Ref: "mcp.atlassian"}, + } + deps := ServerDeps{ + HTTPClient: srv.Client(), + Platform: &types.PlatformConfig{TokenEndpoint: srv.URL, AgentIdentity: "agent-cred"}, + } + authFn := buildAuthFn(spec, deps) + + // With an authenticated user in ctx → that user's token. + ctx := auth.WithIdentity(context.Background(), &auth.Identity{Email: "dave@corp.com"}) + tok, err := authFn(ctx) + if err != nil || tok != "tok-for-dave@corp.com" { + t.Fatalf("resolved token = %q err=%v, want dave's token", tok, err) + } + + // No user in ctx → lazy auth-required. + if _, err := authFn(context.Background()); !errors.Is(err, ErrNoToken) { + t.Fatalf("no user in ctx must fail lazily with ErrNoToken, got: %v", err) + } +} diff --git a/forge-core/mcp/platform_token.go b/forge-core/mcp/platform_token.go index c787715..88f3659 100644 --- a/forge-core/mcp/platform_token.go +++ b/forge-core/mcp/platform_token.go @@ -78,58 +78,147 @@ func (p *platformTokenSource) Token(ctx context.Context) (string, error) { } func (p *platformTokenSource) fetch(ctx context.Context) (string, time.Duration, error) { - endpoint := expandEnvVars(p.endpoint) - identity := expandEnvVars(p.identity) + // Agent-principal: no subject in the body (§19 contract). + tok, ttl, status, err := doPlatformTokenRequest(ctx, p.client, p.endpoint, p.identity, p.ref, "") + if err != nil { + return "", 0, err + } + if status != http.StatusOK { + return "", 0, fmt.Errorf("%w: platform token endpoint returned %d for server %q", ErrProtocolError, status, p.ref) + } + return tok, ttl, nil +} + +// doPlatformTokenRequest POSTs the platform token endpoint (§19 contract) +// and returns the access token + TTL on 200, or the raw status on non-200 +// so the caller can classify (agent-principal vs delegated read a non-200 +// differently). Only transport / parse problems return a non-nil err. The +// endpoint + identity are ${VAR}-expanded at request time so a rotated pod +// secret applies without a restart. +// +// Body: {"server": ref} for agent-principal; {"server": ref, "subject": s} +// for the delegated (per-user) path. access_token only — any refresh_token +// is ignored (invariant 8). +func doPlatformTokenRequest(ctx context.Context, client *http.Client, rawEndpoint, rawIdentity, ref, subject string) (token string, ttl time.Duration, status int, err error) { + endpoint := expandEnvVars(rawEndpoint) + identity := expandEnvVars(rawIdentity) if endpoint == "" { - return "", 0, fmt.Errorf("%w: platform.token_endpoint is empty (or its env var is unset)", ErrProtocolError) + return "", 0, 0, fmt.Errorf("%w: platform.token_endpoint is empty (or its env var is unset)", ErrProtocolError) } if identity == "" { - return "", 0, fmt.Errorf("%w: platform.agent_identity is empty (or its env var is unset) — the platform did not materialize the agent credential", ErrProtocolError) + return "", 0, 0, fmt.Errorf("%w: platform.agent_identity is empty (or its env var is unset) — the platform did not materialize the agent credential", ErrProtocolError) } - body, err := json.Marshal(map[string]string{"server": p.ref}) + payload := map[string]string{"server": ref} + if subject != "" { + payload["subject"] = subject + } + body, err := json.Marshal(payload) if err != nil { - return "", 0, err + return "", 0, 0, err } req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, strings.NewReader(string(body))) if err != nil { - return "", 0, err + return "", 0, 0, err } req.Header.Set("Content-Type", "application/json") req.Header.Set("Accept", "application/json") req.Header.Set("Authorization", "Bearer "+identity) - client := p.client if client == nil { client = &http.Client{Timeout: 15 * time.Second} } resp, err := client.Do(req) if err != nil { - return "", 0, fmt.Errorf("%w: platform token request failed: %v", ErrTransportUnavailable, err) + return "", 0, 0, fmt.Errorf("%w: platform token request failed: %v", ErrTransportUnavailable, err) } defer func() { _ = resp.Body.Close() }() data, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) if resp.StatusCode != http.StatusOK { - return "", 0, fmt.Errorf("%w: platform token endpoint returned %d for server %q: %s", - ErrProtocolError, resp.StatusCode, p.ref, strings.TrimSpace(string(data))) + return "", 0, resp.StatusCode, nil // caller classifies the non-200 } var out struct { AccessToken string `json:"access_token"` ExpiresIn int `json:"expires_in"` - // A refresh_token here would violate invariant 8 — the platform - // must never send one; if it does, it is deliberately ignored. } if err := json.Unmarshal(data, &out); err != nil { - return "", 0, fmt.Errorf("%w: parsing platform token response: %v", ErrProtocolError, err) + return "", 0, resp.StatusCode, fmt.Errorf("%w: parsing platform token response: %v", ErrProtocolError, err) } if out.AccessToken == "" { - return "", 0, fmt.Errorf("%w: platform token response carried no access_token", ErrProtocolError) + return "", 0, resp.StatusCode, fmt.Errorf("%w: platform token response carried no access_token", ErrProtocolError) } - ttl := defaultPlatformTokenTTL + ttl = defaultPlatformTokenTTL if out.ExpiresIn > 0 { ttl = time.Duration(out.ExpiresIn) * time.Second } - return out.AccessToken, ttl, nil + return out.AccessToken, ttl, resp.StatusCode, nil +} + +// delegatedTokenSource is the managed resolver for auth.type=user (#317): +// a per-REQUESTING-USER access token from the platform token endpoint +// (delegated body {server, subject}), cached per subject so distinct users +// never share a token. A platform 401/403/404 means "no grant for this +// user yet" → ErrNoToken (auth-required), keeping the server lazy and +// non-blocking until the platform-side consent flow produces the grant. +// The refresh token never reaches the agent (invariant 8). +type delegatedTokenSource struct { + endpoint string + identity string + ref string + client *http.Client + + mu sync.Mutex + cache map[string]cachedToken // key: subject +} + +type cachedToken struct { + token string + expiresAt time.Time +} + +func newDelegatedTokenSource(cfg PlatformSourceConfig) *delegatedTokenSource { + return &delegatedTokenSource{ + endpoint: cfg.TokenEndpoint, + identity: cfg.AgentIdentity, + ref: cfg.Ref, + client: cfg.HTTPClient, + } +} + +// TokenForSubject returns a valid access token for the requesting user, +// fetching from the platform when the per-subject cache is empty/expiring. +func (d *delegatedTokenSource) TokenForSubject(ctx context.Context, subject string) (string, error) { + if subject == "" { + return "", fmt.Errorf("%w: delegated token requires a requesting-user subject", ErrNoToken) + } + // Fast path: a cached, unexpired token for this subject. The lock is + // NOT held across the network fetch, so a slow fetch for user A never + // blocks user B — the multi-user path is the whole point of #317. + d.mu.Lock() + if c, ok := d.cache[subject]; ok && c.token != "" && time.Now().Before(c.expiresAt.Add(-platformTokenSkew)) { + d.mu.Unlock() + return c.token, nil + } + d.mu.Unlock() + + tok, ttl, status, err := doPlatformTokenRequest(ctx, d.client, d.endpoint, d.identity, d.ref, subject) + if err != nil { + return "", err + } + if status != http.StatusOK { + if status == http.StatusUnauthorized || status == http.StatusForbidden || status == http.StatusNotFound { + return "", fmt.Errorf("%w: no platform grant for subject %q on server %q yet — awaiting the delegated consent flow (#317)", ErrNoToken, subject, d.ref) + } + return "", fmt.Errorf("%w: platform token endpoint returned %d for server %q (subject %q)", ErrProtocolError, status, d.ref, subject) + } + + d.mu.Lock() + if d.cache == nil { + d.cache = map[string]cachedToken{} + } + d.cache[subject] = cachedToken{token: tok, expiresAt: time.Now().Add(ttl)} + d.mu.Unlock() + return tok, nil } // expandEnvVars resolves ${VAR}/$VAR against the process env at USE time — diff --git a/forge-core/mcp/platform_token_test.go b/forge-core/mcp/platform_token_test.go index 8d15f32..3b2b987 100644 --- a/forge-core/mcp/platform_token_test.go +++ b/forge-core/mcp/platform_token_test.go @@ -155,13 +155,19 @@ func TestNewServer_PlatformAndUserRules(t *testing.T) { t.Fatal("user + required:true must be rejected") } u.Required = false - if _, err := NewServer(u, deps); err != nil { - t.Fatalf("lazy user server must construct: %v", err) + // #317: type=user now resolves via the platform token endpoint, so it + // requires the platform block (same as type=platform). + if _, err := NewServer(u, deps); err == nil { + t.Fatal("user without a platform block must fail construction (resolves via the platform endpoint)") + } + if _, err := NewServer(u, depsWithPlatform); err != nil { + t.Fatalf("lazy user server (with platform block) must construct: %v", err) } - fn := buildAuthFn(u, deps) + // No requesting user in ctx → auth-required (lazy), never a token. + fn := buildAuthFn(u, depsWithPlatform) if _, err := fn(context.Background()); !errors.Is(err, ErrNoToken) { - t.Fatalf("user auth must fail lazily with ErrNoToken, got: %v", err) + t.Fatalf("user auth with no requesting user must fail lazily with ErrNoToken, got: %v", err) } } diff --git a/forge-core/mcp/server.go b/forge-core/mcp/server.go index 432dd76..11355c8 100644 --- a/forge-core/mcp/server.go +++ b/forge-core/mcp/server.go @@ -11,6 +11,7 @@ import ( "testing" "time" + "github.com/initializ/forge/forge-core/auth" "github.com/initializ/forge/forge-core/runtime" "github.com/initializ/forge/forge-core/types" ) @@ -178,6 +179,12 @@ func NewServer(spec types.MCPServer, deps ServerDeps) (*Server, error) { if spec.Required { return nil, fmt.Errorf("%w: server %q: auth.type=user cannot be required:true — delegated identity connects lazily after consent", ErrProtocolError, spec.Name) } + // #317: the delegated token resolves via the platform token + // endpoint (per-user, {server, subject}) — so the platform block + // is the contract, same as type=platform. + if deps.Platform == nil || deps.Platform.TokenEndpoint == "" { + return nil, fmt.Errorf("%w: server %q: auth.type=user requires the top-level platform block (delegated tokens resolve via the platform token endpoint)", ErrProtocolError, spec.Name) + } } } logger := deps.Logger @@ -590,13 +597,28 @@ func buildAuthFn(spec types.MCPServer, deps ServerDeps) AuthTokenFunc { }) return src.Token case "user": - // Delegated user identity — no grant exists until the - // platform-side consent flow (#317) completes. Fail each attempt - // with an actionable error; the server is non-Required so it + // Delegated user identity (#317): resolve a per-REQUESTING-USER + // token from the platform token endpoint. Lazy by design — until a + // request carries an authenticated user, and until the platform has + // a grant for that user, this fails with ErrNoToken so the server // never blocks startup. + ref := spec.Auth.Ref + if ref == "" { + ref = spec.Name + } + src := newDelegatedTokenSource(PlatformSourceConfig{ + TokenEndpoint: deps.Platform.TokenEndpoint, + AgentIdentity: deps.Platform.AgentIdentity, + Ref: ref, + HTTPClient: deps.HTTPClient, + }) serverName := spec.Name - return func(_ context.Context) (string, error) { - return "", fmt.Errorf("%w: server %q uses delegated user identity — no user grant yet (the platform consent flow completes it; writes are lazy by design)", ErrNoToken, serverName) + return func(ctx context.Context) (string, error) { + subject := delegatedSubject(ctx) + if subject == "" { + return "", fmt.Errorf("%w: server %q uses delegated user identity but no requesting user is in context — it connects lazily under a user's session, never at startup", ErrNoToken, serverName) + } + return src.TokenForSubject(ctx, subject) } } // Defense in depth — NewServer should have rejected an unknown @@ -611,6 +633,22 @@ func buildAuthFn(spec types.MCPServer, deps ServerDeps) AuthTokenFunc { } } +// delegatedSubject extracts the requesting user's stable identifier from +// the authenticated request context for the type=user resolver (#317). +// Email is preferred (portable, matches the §18 email-keyed model), then +// the user id. Empty when there is no authenticated user in ctx (e.g. a +// connection established at startup with no request behind it). +func delegatedSubject(ctx context.Context) string { + id := auth.IdentityFromContext(ctx) + if id == nil { + return "" + } + if id.Email != "" { + return id.Email + } + return id.UserID +} + // getenv is overridable for tests. var getenv = func(k string) string { if k == "" { From 6582f78389b5a849c167c903d7649d64224e4610 Mon Sep 17 00:00:00 2001 From: MK Date: Fri, 17 Jul 2026 09:43:08 -0400 Subject: [PATCH 2/2] =?UTF-8?q?fix(mcp):=20#327=20review=20=E2=80=94=20cac?= =?UTF-8?q?he=20eviction=20+=20isolation-boundary=20docs=20caveat?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finding 1 (docs): add the connection-sharing caveat. type=user resolves a per-user token per CALL, but the underlying MCP connection is still shared (per-subject connection isolation is the follow-up). A stateless server that re-authorizes per request honors the per-call token; a session-stateful server that binds identity at initialize may not, so the docs now say to confirm per-request re-auth before relying on this for hard isolation. Finding 2: bound the per-subject cache. Evict the stale entry on the fast-path read, and opportunistically sweep expired entries on each write — so the map can't grow unbounded with one-shot users' stale tokens, and sensitive tokens aren't held past use. No background goroutine. Findings 3 (per-subject singleflight) + the email-in-errors nit noted on the threads as follow-ups; the "no grant yet" message keeps the subject because the operator needs to know who must consent. --- docs/mcp/configuration.md | 10 ++++++++++ forge-core/mcp/platform_token.go | 21 +++++++++++++++++---- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/docs/mcp/configuration.md b/docs/mcp/configuration.md index db5cb61..770129a 100644 --- a/docs/mcp/configuration.md +++ b/docs/mcp/configuration.md @@ -111,6 +111,16 @@ mcp: - **Egress:** the platform token-endpoint host is auto-merged into the allowlist. +> ⚠️ **Isolation boundary (today).** `type: user` currently resolves a +> per-user **token on each call**; the underlying MCP **connection is still +> shared** across users (per-subject connection isolation is a follow-up). +> A **stateless** MCP server that re-authorizes on every request honors the +> per-call token, so this gives real per-user isolation. A **session-stateful** +> server that binds identity at `initialize` may not re-scope the session +> when the per-call token changes — so confirm your MCP server re-authorizes +> per request before relying on `type: user` for hard isolation against such +> a server. + > The platform materializes both the `platform` block and the per-identity > server entries (same URL, split by `type`). For the **standalone** (no > platform) equivalents, use `type: oauth` — 3-legged `forge mcp login` for a diff --git a/forge-core/mcp/platform_token.go b/forge-core/mcp/platform_token.go index 88f3659..1c6e8ea 100644 --- a/forge-core/mcp/platform_token.go +++ b/forge-core/mcp/platform_token.go @@ -195,9 +195,14 @@ func (d *delegatedTokenSource) TokenForSubject(ctx context.Context, subject stri // NOT held across the network fetch, so a slow fetch for user A never // blocks user B — the multi-user path is the whole point of #317. d.mu.Lock() - if c, ok := d.cache[subject]; ok && c.token != "" && time.Now().Before(c.expiresAt.Add(-platformTokenSkew)) { - d.mu.Unlock() - return c.token, nil + if c, ok := d.cache[subject]; ok { + if c.token != "" && time.Now().Before(c.expiresAt.Add(-platformTokenSkew)) { + d.mu.Unlock() + return c.token, nil + } + // Evict the stale entry — don't hold a sensitive token past use + // (#327 review finding 2). + delete(d.cache, subject) } d.mu.Unlock() @@ -216,7 +221,15 @@ func (d *delegatedTokenSource) TokenForSubject(ctx context.Context, subject stri if d.cache == nil { d.cache = map[string]cachedToken{} } - d.cache[subject] = cachedToken{token: tok, expiresAt: time.Now().Add(ttl)} + // Opportunistic sweep so the map can't grow unbounded with one-shot + // users' stale tokens — a background-free TTL bound (#327 review). + now := time.Now() + for s, c := range d.cache { + if !now.Before(c.expiresAt) { + delete(d.cache, s) + } + } + d.cache[subject] = cachedToken{token: tok, expiresAt: now.Add(ttl)} d.mu.Unlock() return tok, nil }