fix(egress): stop resolving the egress vault id across identifier namespaces

[go-faustino]

A user completes the browser consent for a per-user egress MCP server, the consent succeeds, and then every per-user egress server returns an empty tools/list to their IDE client. Nothing is logged. The vend simply returns no token and the caller sees zero tools, so there is no signal anywhere pointing at the cause.

The cause is that two code paths resolve the per-user egress vault key through different identifier namespaces, so one human ends up with two parallel vault identities. The consent-write path resolves the OIDC sub. The vend path, when the caller is a gateway-minted self-signed token that carries no egress_user claim, falls through to that token's sub — which is the login username by construction, and on Keycloak the login username is the user's email address. The token is written to one bucket and read from another, and the miss is indistinguishable from "never consented".

This is issue #1695. That issue was closed as completed with no linked fix; a maintainer may well have closed it alongside related work, but the code on main still exhibits the problem, which is what this change addresses.

This builds on two already-merged PRs rather than reverting either of them. #1491 established that the vault keys on the OIDC sub across both consent and vend. #1530 established that the egress_user claim is only trusted from a self_signed token this gateway minted itself, so an external issuer cannot inject a vault key. Both are correct and both are preserved here — the trust gate from #1530 is untouched. The remaining gap is what happens when the gate applies but the claim is absent: resolution kept walking down a chain whose remaining entries are, for that specific token type, the login username.

The resolution chain

Before, in _canonical_egress_user:

egress_user (self_signed only) -> data.sub -> data.subject -> sub -> username -> ""

After:

self_signed:      egress_user -> ""
everything else:  data.sub -> data.subject -> sub -> username -> ""

The second line is byte-for-byte what it was. Only the self_signed case changes, and only when the claim is missing. A correctly minted self-signed token still resolves to its egress_user, and non-OIDC callers with only a username still key on the username exactly as documented.

The vend in registry/api/egress_auth_routes.py changes from

sub = claims.get("egress_user") or claims.get("sub") or ""

to reading only egress_user, and then, after the existing per-user check, refusing an empty value with a warning that names the server and the reason.

On the non-per-user sub fallback

The fallback is gone entirely rather than retained for non-per-user callers. Reading the surrounding code, sub is only ever consumed on per-user branches: the very next check returns consent_required=True for any caller whose auth_method is not per-user, before sub is used for anything. A non-per-user fallback would therefore be dead code that only invites a future reader to reintroduce the namespace crossing.

Deliberate behaviour change

Tokens minted before the egress_user claim existed will now be refused instead of silently keyed on the wrong bucket. This is intentional. Those tokens have a short TTL — the UI states 8 hours — so the blast radius is bounded to one token lifetime after upgrade, and the next mint carries the claim, which makes the condition self-healing. Crucially, the refusal is now visible: it logs a warning naming the server, where previously the same situation produced an empty tool list and complete silence. Trading one bounded token lifetime of visible, self-correcting refusals for the removal of a silent permanent misroute is the better failure mode, so this does not add a configuration flag to opt out.

What this does not change

registry/api/egress_auth_routes.py around line 173, in _resolve_target_principal, has a third instance of the same pattern: user_context.get("egress_user") or user_context.get("username") or "". It is left alone deliberately. That line is on the PAT submission and consent principal path, not the vend, and it resolves against a session-derived user_context rather than an mcp-proxy token, so the reasoning about which namespace the remaining candidate belongs to is different and needs its own analysis. Changing it speculatively risks breaking PAT flows for a defect that has not been demonstrated there. It is worth a follow-up on its own.

Tests

tests/auth_server/unit/test_egress_mint.py gains five cases in the existing TestCanonicalEgressUserPaths suite: a self-signed token with no egress_user but a username-shaped data.sub must resolve to ""; the same with data.subject, top-level sub and username present, since none of those is an OIDC sub on that token type either; a regression guard that a self-signed token with a valid egress_user still resolves to it; a regression guard that non-self-signed OIDC callers still resolve through data.sub and data.subject; and a regression guard that a non-OIDC caller with only a username still keys on the username.

tests/unit/egress_auth/test_internal_egress_token_route.py gains a case asserting that a per-user caller with no egress_user claim gets consent_required=True with no token, no service call, and a warning naming the server, using caplog.

The shared _claims() helper in test_internal_egress_token_route.py and test_pat_vend.py now includes egress_user alongside sub, so those fixtures reflect the shape of a token the current auth-server actually mints. Those suites test upstream binding, path normalisation and PAT vending, not the removed fallback, and all their assertions are unchanged.

Counts from a local run with the repository's pytest:

• tests/auth_server/unit/test_egress_mint.py plus tests/unit/egress_auth/: 307 passed before, 313 passed after.
• Three of the six new cases fail against the unmodified source and pass with it; the other three are regression guards that pass in both states, which is their purpose.
• Full run of tests/auth_server/ and tests/unit/: 7344 passed, 1 failed, 8 skipped. The single failure is tests/unit/audit/test_routes.py::TestAdminOnlyAccess::test_rejects_non_admin_users, a Hypothesis DeadlineExceeded flake (the test took 5008 ms against a 200 ms deadline, reported as FlakyFailure after it did not reproduce on the retry). It is unrelated to this change, touches no egress code, and passes on its own both with and without these edits.

docs/egress-credential-vault.md is updated to record both fail-closed points in the section describing the canonical user_id.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!