Add RegisterHandlers and wire embedded AS routes on vMCP mux by jhrozek · Pull Request #4348 · stacklok/toolhive

jhrozek · 2026-03-24T16:50:14Z

Summary

The vMCP server needs to host both RFC 9728 protected resource metadata and embedded authorization server (AS) OAuth/OIDC endpoints on the same HTTP mux. The previous /.well-known/ catch-all registration would conflict with AS discovery routes.
Replace /.well-known/ catch-all with explicit /.well-known/oauth-protected-resource path registrations so the AS can own its own /.well-known/ endpoints without conflicts.
Add RegisterHandlers method on EmbeddedAuthServer that registers /.well-known/openid-configuration, /.well-known/oauth-authorization-server, /.well-known/jwks.json, and /oauth/ routes.
Add AuthServer field to vmcpserver.Config with nil-guarded route registration in the mux setup.

Fixes #4141

Type of change

New feature

Test plan

Linting (task lint-fix)
Unit tests (task test)

Changes

File	Change
`pkg/authserver/runner/embeddedauthserver.go`	Add `RegisterHandlers(mux)` method
`pkg/vmcp/server/server.go`	Add `AuthServer` field, explicit `.well-known/` paths, Mode B route registration

Special notes for reviewers

This is part of a stacked PR series for the embedded auth server feature. This PR adds only the server-side wiring points. The construction of the EmbeddedAuthServer and config loading come in later PRs:

scaffolding-3: upstream_inject strategy types + cross-cutting validation
scaffolding-4: operator reconciler condition for AuthServerConfig
scaffolding-5: converter + CLI auth server config loading

Generated with Claude Code

codecov · 2026-03-24T16:56:16Z

Codecov Report

❌ Patch coverage is 70.58824% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.74%. Comparing base (4838bb4) to head (0bd1c80).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/vmcp/server/server.go	50.00%	2 Missing and 1 partial ⚠️
pkg/authserver/runner/embeddedauthserver.go	90.00%	0 Missing and 1 partial ⚠️
pkg/runner/runner.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4348      +/-   ##
==========================================
- Coverage   68.86%   68.74%   -0.13%     
==========================================
  Files         479      479              
  Lines       48505    48530      +25     
==========================================
- Hits        33404    33362      -42     
+ Misses      12347    12328      -19     
- Partials     2754     2840      +86

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

pkg/authserver/runner/embeddedauthserver.go

Phase 2 of the vMCP embedded authorization server (#4141): - Add RegisterHandlers(mux) to EmbeddedAuthServer — the AS owns its route paths (/oauth/, /.well-known/openid-configuration, etc.) - Add AuthServer field to vMCP server Config - Replace /.well-known/ catch-all with explicit registrations: /.well-known/oauth-protected-resource (exact + subpath) is always registered; Mode B conditionally adds AS discovery/JWKS routes - No commands.go changes — CLI path deferred to Phase 4 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The proxy runner and vMCP server both registered the same four authorization server routes independently. Extract a Routes() method on EmbeddedAuthServer that returns the canonical route map, and have both RegisterHandlers and the proxy runner consume it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

jerm-dro · 2026-03-24T22:23:13Z

question: @jhrozek — AuthInfoHandler and AuthServer are semantically coupled: when an embedded AS is present, the protected resource metadata should advertise that AS's endpoints. Currently they're set independently on Config, which means nothing prevents them from diverging (e.g., AuthInfoHandler pointing to a different AS than the embedded one, or AuthServer being set without any protected resource metadata).

As scaffolding PRs 3-5 land, the auth surface on Config will be four fields: AuthMiddleware, AuthzMiddleware, AuthInfoHandler, and AuthServer. Do you plan to group these into an AuthConfig struct (or have AuthServer produce the AuthInfoHandler itself)? That would:

Make the invariant explicit — embedded AS implies its own protected resource metadata
Reduce the auth-related field sprawl on server.Config, which is already at 15+ fields (see vMCP anti-pattern Bump golangci/golangci-lint-action from 2f856675483cb8b9378ee77ee0beb67955aca9d7 to 4696ba8babb6127d732c3c6dde519db15edab9ea #3, god object)
Let the mux setup in Handler() delegate to a single authConfig.RegisterRoutes(mux) instead of two independent nil checks

Not blocking — just want to understand the plan before the follow-up PRs land.

jhrozek · 2026-03-25T13:18:31Z

question: @jhrozek — AuthInfoHandler and AuthServer are semantically coupled: when an embedded AS is present, the protected resource metadata should advertise that AS's endpoints. Currently they're set independently on Config, which means nothing prevents them from diverging (e.g., AuthInfoHandler pointing to a different AS than the embedded one, or AuthServer being set without any protected resource metadata).

As scaffolding PRs 3-5 land, the auth surface on Config will be four fields: AuthMiddleware, AuthzMiddleware, AuthInfoHandler, and AuthServer. Do you plan to group these into an AuthConfig struct (or have AuthServer produce the AuthInfoHandler itself)? That would:

yes, I'd like to land a couple of more PRs but I do have a refactoring planned as the last one. Let me push it up in pauses betweeen review iterations as a draft so you can review.

PR #4348 (f64d807) replaced the /.well-known/ catch-all mux pattern with explicit exact registrations to prevent the auth server from intercepting /.well-known/oauth-protected-resource. This broke RFC 8414 Section 3.1 discovery for path-based issuers: clients construct /.well-known/oauth-authorization-server/{issuer-path} but the exact pattern only matched /.well-known/oauth-authorization-server. Register trailing-slash prefix variants on both the http.ServeMux (Routes()) and chi router (WellKnownRoutes) so subpaths are routed to the discovery handlers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

PR #4348 (f64d807) replaced the /.well-known/ catch-all mux pattern with explicit exact registrations to prevent the auth server from intercepting /.well-known/oauth-protected-resource. This broke RFC 8414 Section 3.1 discovery for path-based issuers: clients construct /.well-known/oauth-authorization-server/{issuer-path} but the exact pattern only matched /.well-known/oauth-authorization-server. Register trailing-slash prefix variants on both the http.ServeMux (Routes()) and chi router (WellKnownRoutes) so subpaths are routed to the discovery handlers. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

jhrozek requested review from ChrisJBurns, JAORMX, amirejaz, jerm-dro and yrobla as code owners March 24, 2026 16:50

jhrozek mentioned this pull request Mar 24, 2026

Add upstream_inject strategy type and auth server validation #4349

Merged

3 tasks

github-actions bot added the size/XS Extra small PR: < 100 lines changed label Mar 24, 2026

tgrunnagle previously approved these changes Mar 24, 2026

View reviewed changes

pkg/authserver/runner/embeddedauthserver.go Show resolved Hide resolved

jhrozek and others added 2 commits March 24, 2026 18:13

jhrozek dismissed tgrunnagle’s stale review via 0bd1c80 March 24, 2026 19:07

jhrozek force-pushed the vmcp-add-as-scaffolding-2 branch from afc069c to 0bd1c80 Compare March 24, 2026 19:07

jhrozek requested a review from lujunsan as a code owner March 24, 2026 19:07

github-actions bot added size/S Small PR: 100-299 lines changed and removed size/XS Extra small PR: < 100 lines changed size/S Small PR: 100-299 lines changed labels Mar 24, 2026

tgrunnagle approved these changes Mar 24, 2026

View reviewed changes

jhrozek merged commit f64d807 into main Mar 24, 2026
78 of 79 checks passed

jhrozek deleted the vmcp-add-as-scaffolding-2 branch March 24, 2026 19:59

jhrozek mentioned this pull request Mar 27, 2026

Support RFC 8414 path-based issuer discovery URLs #4406

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add RegisterHandlers and wire embedded AS routes on vMCP mux#4348

Add RegisterHandlers and wire embedded AS routes on vMCP mux#4348
jhrozek merged 2 commits intomainfrom
vmcp-add-as-scaffolding-2

jhrozek commented Mar 24, 2026 •

edited

Loading

Uh oh!

codecov bot commented Mar 24, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

jerm-dro commented Mar 24, 2026

Uh oh!

jhrozek commented Mar 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

jhrozek commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Type of change

Test plan

Changes

Special notes for reviewers

Uh oh!

codecov bot commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

jerm-dro commented Mar 24, 2026

Uh oh!

jhrozek commented Mar 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jhrozek commented Mar 24, 2026 •

edited

Loading

codecov bot commented Mar 24, 2026 •

edited

Loading