Add default OBO handler hooks and vMCP/proxy converter stubs

[tgrunnagle]

Summary

This is the second of four flat technical tasks for the OSS on-behalf-of (OBO) work. It stands up the default stub infrastructure for a new obo external auth type at the three dispatch layers (operator, vMCP converter, proxy runtime), plus the Go constant the other layers reference. After this change, an out-of-tree build can register a real OBO implementation by calling three setter functions; an upstream-only build hits the defaults and surfaces a sentinel obo.ErrEnterpriseRequired error at every dispatch point.

The CRD enum is not widened in this PR — spec.type: obo is still rejected at the apiserver layer. The constant, handler hooks, middleware-factory entry, and converter stub live as dark infrastructure that activates the moment the CRD admission task (#5329) merges.

• Why: enable out-of-tree builds to register a real OBO implementation without forking, while keeping a clean, function-pointer override hook surface in upstream. Defining the hooks now lets the dispatch-wiring task (Wire OBO dispatch arms and reconciler branch; add integration tests #5328) land in isolation against a stable, reviewed surface.
• What:
  • Adds ExternalAuthTypeOBO constant in mcpexternalauthconfig_types.go and minimal no-op exhaustive linter arms.
  • Adds the new pkg/auth/obo/ package — leaf home for ErrEnterpriseRequired, MiddlewareType, the default CreateMiddleware redirector (503 stub), DefaultFactory, and RegisterFactory. Wires one new entry into pkg/runner/middleware.go.
  • Adds the OBOHandler struct, RegisterOBOHandler setter (panics on partial registration), and OBOValidate / OBOSecretEnvVars / OBOApplyRunConfig wrappers in controllerutil/tokenexchange.go. Reads/writes guarded by a sync.RWMutex.
  • Adds OBOConverter in pkg/vmcp/auth/converters/obo.go and registers it as a built-in in NewRegistry().
  • Adds StrategyTypeOBO constant in pkg/vmcp/auth/types/types.go.
  • Unit tests cover every default-stub behavior, the last-write-wins setter semantics, and the panic-on-partial-registration contract.

Closes #5327

Type of change

• New feature

Test plan

• Unit tests (task test)
• Linting (task lint-fix)
• Race-detected unit tests for the touched packages (go test -race ./pkg/auth/obo/... ./cmd/thv-operator/pkg/controllerutil/... ./pkg/runner/...)

API Compatibility

• This PR does not break the v1beta1 API, OR the api-break-allowed label is applied and the migration guidance is described above.

The new ExternalAuthTypeOBO Go constant is purely additive and is not yet present in the CRD enum, so the apiserver behavior is unchanged.

Changes

File	Change
cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go	Add ExternalAuthTypeOBO constant; add no-op Validate() switch arm with TODO(#5328) pointer for the reconcile-loop dispatch site and TODO(#5329) pointer for the biconditional/unauthenticated checks
pkg/auth/obo/errors.go	New leaf home for ErrEnterpriseRequired (the cross-layer sentinel imported by both operator and vMCP code paths)
pkg/auth/obo/middleware.go	MiddlewareType constant, CreateMiddleware as a stable redirector over currentFactory (guarded by sync.RWMutex), DefaultFactory (exported so external test code can restore default behavior), and RegisterFactory (panics on nil factory)
pkg/auth/obo/middleware_test.go	Tests covering 503 body/status (locked via exact-match against stubMessage), default factory's runner registration, last-write-wins re-registration, panic-on-nil
pkg/auth/obo/errors_test.go	Sentinel-wrap test: errors.Is works through fmt.Errorf("...: %w", ...)
cmd/thv-operator/pkg/controllerutil/tokenexchange.go	OBOHandler struct, RegisterOBOHandler (panics on any nil func field), OBOValidate / OBOSecretEnvVars / OBOApplyRunConfig wrappers; sync.RWMutex-guarded oboHandler package state; ExternalAuthTypeOBO arm in AddExternalAuthConfigOptions returns obo.ErrEnterpriseRequired (dispatch through the handler hook lands in #5328)
cmd/thv-operator/pkg/controllerutil/tokenexchange_test.go	Tests covering default handler behavior, dispatch through wrappers, last-write-wins re-registration, panic-on-partial-registration
cmd/thv-operator/controllers/virtualmcpserver_deployment.go	Add no-op ExternalAuthTypeOBO arm to getExternalAuthConfigSecretEnvVar switch with TODO(#5328) pointer for the dispatch rewrite
pkg/runner/middleware.go	Add obo.MiddlewareType: obo.CreateMiddleware entry to the factory map
pkg/runner/middleware_test.go	Extend coverage to include the new obo entry; lock the call-after-register contract for obo.CreateMiddleware
pkg/vmcp/auth/converters/obo.go	New OBOConverter implementing StrategyConverter with obo.ErrEnterpriseRequired-wrapped errors
pkg/vmcp/auth/converters/obo_test.go	Tests for stub method behavior and errors.Is propagation
pkg/vmcp/auth/converters/interface.go	Register OBOConverter as built-in in NewRegistry()
pkg/vmcp/auth/converters/registry_test.go	Extend coverage to assert OBOConverter is registered
pkg/vmcp/auth/types/types.go	Add StrategyTypeOBO constant; extend BackendAuthStrategy.Type doc to list "obo"
docs/operator/crd-api.md	Regenerated CRD reference docs to include the new obo enum value

Does this introduce a user-facing change?

No. The CRD enum still rejects spec.type: obo, so no user-visible behavior changes in upstream-only builds. Once the CRD admission task (#5329) lands, an upstream-only build will surface obo.ErrEnterpriseRequired as status.conditions[Valid] = False with Reason: EnterpriseRequired for obo-typed resources.

Special notes for reviewers

• Dispatch wiring is intentionally deferred: the case ExternalAuthTypeOBO arms in AddExternalAuthConfigOptions and getExternalAuthConfigSecretEnvVar are minimal placeholders to satisfy the exhaustive linter. The next task (Wire OBO dispatch arms and reconciler branch; add integration tests #5328) replaces them with real dispatch into OBOApplyRunConfig / OBOSecretEnvVars, and adds the reconcile-loop branch that maps obo.ErrEnterpriseRequired to Reason: EnterpriseRequired. Every site that Wire OBO dispatch arms and reconciler branch; add integration tests #5328 needs to touch is tagged with TODO(#5328): for grep-discoverability.
• obo.CreateMiddleware is a stable redirector: calls dispatch through currentFactory under an RWMutex on every invocation, so RegisterFactory after the pkg/runner middleware map is built still takes effect. This removes the captured-at-construction-time fragility that an earlier revision of this PR carried (review feedback).
• Partial registration fails loudly: RegisterOBOHandler panics if any of Validate / ApplyRunConfig / SecretEnvVars is nil; obo.RegisterFactory panics on a nil factory. Both panic at init() time in real builds, so a misconfigured out-of-tree wiring surfaces at process start, not at reconcile time.
• Single-slot, last-write-wins: all three setters (RegisterOBOHandler, obo.RegisterFactory, and the Registry.Register map for the converter) accept double-registration without panic, matching the existing pkg/config.RegisterProviderFactory precedent. Tests cover this.
• Blocks: Wire OBO dispatch arms and reconciler branch; add integration tests #5328 (dispatch wiring) and Admit obo in MCPExternalAuthConfig CRD enum and regenerate manifests #5329 (CRD enum admission) both depend on this PR being merged first.

Generated with Claude Code

[github-actions]

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details.

---

This review will be automatically dismissed once you add the justification section.

[codecov]

Codecov Report

❌ Patch coverage is 90.16393% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.39%. Comparing base (8d22ac5) to head (a480c64).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...perator/api/v1beta1/mcpexternalauthconfig_types.go	0.00%	2 Missing ⚠️
...perator/controllers/virtualmcpserver_deployment.go	0.00%	2 Missing ⚠️
...d/thv-operator/pkg/controllerutil/tokenexchange.go	93.33%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5338      +/-   ##
==========================================
- Coverage   68.41%   68.39%   -0.03%     
==========================================
  Files         621      624       +3     
  Lines       63278    63432     +154     
==========================================
+ Hits        43293    43385      +92     
- Misses      16757    16809      +52     
- Partials     3228     3238      +10     

☔ 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.

PR size has been reduced below the XL threshold. Thank you for splitting this up!

[github-actions]

✅ PR size has been reduced below the XL threshold. The size review has been dismissed and this PR can now proceed with normal review. Thank you for splitting this up!

[tgrunnagle]

Multi-Agent Consensus Review

Agents consulted: operator/CRD, vMCP/auth, Go style, security, tests, general quality (codex skipped — CLI not installed)

Consensus Summary

#	Finding	Consensus	Severity	Action
1	Validate() returns nil for OBO — defense-in-depth fail-open	9/10	MEDIUM	Fix
2	Three OBO switch arms behave inconsistently (error vs nil)	9/10	MEDIUM	Fix
3	Missing unit tests for the three new OBO switch arms	8/10	MEDIUM	Fix
4	Package-level OBO state mutated without sync — race risk	8/10	MEDIUM	Discuss
5	stubMessage comment claims exported but is unexported	8/10	MEDIUM	Fix
6	pkg/vmcp -> cmd/thv-operator/pkg/controllerutil cross-layer import	8/10	MEDIUM	Discuss
7	Init-order comment imprecise (function-call time, not runner-construction)	7/10	MEDIUM	Fix
8	validateTypeConfigConsistency missing OBO biconditional	7/10	MEDIUM	Discuss
9	OBOHandler partial registration risks nil-deref at dispatch	7/10	MEDIUM	Discuss
10	PR body "Changes" table lists 4 files not in diff (see note below)	7/10	MEDIUM	Fix
11	OBOValidate / OBOSecretEnvVars / OBOApplyRunConfig wrappers have zero in-tree callers	7/10	LOW	Discuss
12	OBOConverterStub "Stub" suffix inconsistent with sibling naming	7/10	LOW	Discuss
13	TestStubHandler body substring OR is redundant	7/10	LOW	Fix
14	ErrEnterpriseRequired message leaks Go package path into user-visible Status	7/10	LOW	Fix
15	503 stub body discloses OBO type to unauthenticated callers	7/10	LOW	Discuss
16	TODO(#5328) format inconsistency hurts discoverability	7/10	LOW	Fix

Overall

The PR does exactly what the body advertises: three independent single-slot last-write-wins registration points across operator, vMCP converter, and proxy middleware, with a sentinel ErrEnterpriseRequired everywhere the default fires. Architecture is sound; CRD enum is correctly left in place so nothing user-facing changes today.

The central design choice — every dispatch site fails closed with ErrEnterpriseRequired — is undermined in three places where the new OBO arm returns nil instead of routing through the registered handler: MCPExternalAuthConfig.Validate(), getExternalAuthConfigSecretEnvVar, and the wrappers OBOValidate/OBOSecretEnvVars/OBOApplyRunConfig themselves have zero in-tree callers. The comments justify this with "the CRD enum rejects 'obo'" — but the defense-in-depth Go-level Validate() exists exactly to catch what the enum misses, and once #5329 widens the enum the gate is gone. The contract should be self-enforcing now so #5328 doesn't have to remember to rewire three independent sites correctly.

Two cross-cutting items: a new cross-layer import pkg/vmcp/auth/converters/obo.go -> cmd/thv-operator/pkg/controllerutil (no other converter in the package depends on operator internals — moving ErrEnterpriseRequired to a leaf package keeps direction clean), and a comment on obo.CreateMiddleware that says values are captured "at runner-construction time" when GetSupportedMiddlewareFactories() actually re-reads on every call (capture-once comes from NewRunner).

Test coverage is solid for the wrappers, absent for the actual new switch arms. A regression there would not break a test.

Meta finding (not inline because it's about the PR body)

Finding #10: The PR body's "Changes" table includes rows for cmd/thv-operator/app/app.go, main.go, and the proxyrunner equivalents described as "mechanical previously-landed refactor". gh pr view shows 14 files in the diff; those four are not among them. Remove the stale row — it invites confusion for reviewers.

---

Generated with Claude Code — multi-agent consensus review.

[tgrunnagle]

Addressed Finding #10 (and gave the PR body a broader refresh while I was in there): removed the four stale cmd/thv-operator/app/app.go / main.go / proxyrunner rows from the Changes table; updated remaining rows to reflect the post-review file list (pkg/auth/obo/errors.go is new, OBOConverterStub renamed to OBOConverter, sync.RWMutex added on registration sites); rewrote the "Init-order caveat" note to describe the new redirector + mutex semantics; replaced the ErrEnterpriseRequired references with obo.ErrEnterpriseRequired.

Two commits in this round, addressing the review:

• b8d97ce — move sentinel to pkg/auth/obo; comment/naming/TODO hygiene (F5, F6, F8 verify, F12, F13, F14, F16)
• 3095318 — synchronize OBO registration sites; panic on partial registration (F4, F7, F9)

Deferred to #5328 with rationale on each thread: F1, F2 (both arms), F3, F11. Deferred F15 (information disclosure) with a follow-up note. F10 (PR body) handled by the body edit above.

go test -race ./pkg/auth/obo/... ./cmd/thv-operator/pkg/controllerutil/... ./pkg/runner/ passes; task lint-fix reports 0 issues.

[jhrozek]

Clean scaffolding PR — dispatch surface for OBO looks solid, tests cover the security-critical paths (503 status locked, downstream never invoked, last-write-wins, panic-on-nil), and the CRD enum genuinely stays closed until #5329. Approving. Two non-blocking questions inline.

The base branch was changed.

[github-actions]

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details.

---

This review will be automatically dismissed once you add the justification section.

PR size has been reduced below the XL threshold. Thank you for splitting this up!

[github-actions]

✅ PR size has been reduced below the XL threshold. The size review has been dismissed and this PR can now proceed with normal review. Thank you for splitting this up!