security: keep machine identity on delegated tokens and the required_relations gate (GHSA-vq29-8q3c-3hrm)

[lakhansamani]

Fixes GHSA-vq29-8q3c-3hrm (high), plus a second instance of the same primitive found in review. Developed in the advisory private fork; opened here to merge and ship in 2.4.1.

The vulnerability

An RFC 8693 delegated token minted from a service-account subject dropped the login_method claim. service.resolveFgaCaller treats a missing login_method as a human user, so a machine identity laundered into user:<sub> — flipping OpenFGA decisions deny→allow and slipping past every login_method-keyed guard.

The reported root cause was wrong

The report framed this as self-delegation (subject == actor) and proposed rejecting that shape. That is a symptom patch: the laundering lives in CreateDelegatedAccessToken omitting the claim, so it applies identically to the multi-hop A→B agent chain the design explicitly supports and which the proposed check does not catch. Both shapes are asserted in tests; both fail against pre-fix code.

The self-delegation rejection is deliberately not added — once the subject keeps its real identity, subject == actor mints an ordinary attenuated, resource-bound machine token.

Second instance, found in review

enforceRequiredRelations resolved the caller and then discarded caller.subject, hardcoding "user:"+userID. machineFgaSubject never ran on that surface, so a machine identity was answered as a human user there too — with no delegation at all: a plain client_credentials token presented to validate_jwt_token with required_relations was satisfied by a tuple written for user:<service-account-row-id>, while check_permissions denied the identical token. That is the "two answers to one authority question" the function's own doc comment warns about, and a gateway gating on required_relations would admit requests the permission API refuses.

Classification is now extracted into fgaSubjectFor, so all three FGA decision surfaces share one source of truth.

Three claims in the report that are wrong

• "Silent revocation misfire" — false. Machine tokens are registered at service_account:<sa.ID> / access_token_<nonce> (token.go:1138,1164), exactly what delegationSessionIsLive looks up. TestDelegatedMachineTokenRevocationWorks passes on unpatched code.
• "Bare-origin resource is out of scope" — by design. delegated_access_token.go states it verbatim; first-party acceptance is the documented contract.
• requireOrgAdmin bypass — zero impact. The membership lookup already denies, since a laundered token's sub is a client id.

Version range correction

Submitted as > 2.3.0-rc.10. Wrong in both directions: 2.3.0-rc.10 is affected, and 2.3.0 is not — it is not a descendant of 2.3.0-rc.10 (divergent release lines) and contains no token-exchange code at all. Affected: 2.3.0-rc.10, and 2.4.0-rc.0 … 2.4.0.

Backward compatibility

CreateDelegatedAccessToken has exactly one caller, so authorization_code, implicit, refresh_token, client_credentials, OIDC id_token/userinfo/discovery/JWKS, SAML, SCIM, magic link and social login are untouched. User-subject delegation is unchanged — still no login_method, still user:<sub> — and asserted so a future change cannot stamp it.

Intentional changes:

1. A machine-subject delegated token resolves to service_account:<client_id>. Tuples written against user:<sa-row-id> stop matching — the vulnerability closing.
2. A chained machine-subject re-exchange now succeeds where it returned 400. Previously the token carried no login_method, so hop 2 took the user branch, did GetUserByID(<service-account row id>), found nothing and rejected — machine chains only ever worked for one hop. Still bounded: subject liveness re-checked per hop, monotonic scope attenuation, maxActChainDepth. Pinned by TestChainedMachineSubjectReExchange.

resolveFgaCaller now retains actorID in the machine branch. It dropped it before, on reasoning true of client_credentials but false of a machine-subject delegation, which now reaches that branch — dropping it would collapse authority from perms(agent) ∩ perms(subject) to perms(subject), trading identity laundering for privilege widening. Guarded by a test.

Verification

go build / go vet clean; make test 0 failures; make smoke all pass (incl. mcp_delegated, oauth_authorization_code_+_PKCE, scim_provisioning, org-admin matrix); make lint clean in changed files. Every regression test confirmed to fail pre-fix.

https://claude.ai/code/session_016dbRdiyrapnT7J3tAyZhdR