fix: re-validate provider per request and classify reloads (#25766)

Refactors the `aibridgeproxyd` provider reload mechanism which was unnecessarily complex.

Also ensures that providers are evaluated on each CONNECT request to prevent interception of requests to (newly) disabled providers; in this case the requests will passthrough unencrypted, by design.
This commit is contained in:
Danny Kopping
2026-05-28 13:22:38 +02:00
committed by GitHub
parent 673709bd34
commit a9f5ed7644
7 changed files with 856 additions and 629 deletions
+46 -49
View File
@@ -31,18 +31,6 @@ import (
agplaibridge "github.com/coder/coder/v2/coderd/aibridge"
)
// ProviderRoute is the routing entry for a single AI provider: the
// instance name (the routing key) and the upstream base URL (the
// source of the MITM allowlist host).
type ProviderRoute struct {
Name string
BaseURL string
}
// RefreshProvidersFunc returns the live provider set used by Reload to
// rebuild the proxy's routing snapshot.
type RefreshProvidersFunc func(ctx context.Context) ([]ProviderRoute, error)
// Known AI provider hosts.
const (
HostAnthropic = "api.anthropic.com"
@@ -161,7 +149,7 @@ type Server struct {
// providerRouter keeps CONNECT matching and provider lookup in sync.
type providerRouter struct {
mitmHosts []string // host:port allowlist for the goproxy condition.
mitmHosts []string // host:port set the goproxy condition matches against.
nameByHost map[string]string // lowercase hostname -> provider name.
}
@@ -218,15 +206,8 @@ type Options struct {
// CertStore is an optional certificate cache for MITM. If nil, a default
// cache is created. Exposed for testing.
CertStore goproxy.CertStorage
// DomainAllowlist seeds the boot-time MITM allowlist. Production
// callers should leave this empty and rely on RefreshProviders;
// tests use it to skip the refresh round-trip.
DomainAllowlist []string
// AIBridgeProviderFromHost seeds the boot-time host -> provider
// name mapping. Required iff DomainAllowlist is non-empty.
AIBridgeProviderFromHost func(host string) string
// UpstreamProxy is the URL of an upstream HTTP proxy to chain tunneled
// (non-allowlisted) requests through. If empty, tunneled requests connect
// (non-provider-host) requests through. If empty, tunneled requests connect
// directly to their destinations.
// Format: http://[user:pass@]host:port or https://[user:pass@]host:port
UpstreamProxy string
@@ -249,7 +230,7 @@ type Options struct {
// If nil, metrics will not be recorded.
Metrics *Metrics
// RefreshProviders, when set, is invoked by Server.Reload to fetch
// the live provider snapshot used to derive the MITM allowlist and
// the live provider snapshot used to derive the MITM host set and
// host -> provider-name routing. Nil disables hot-reload.
RefreshProviders RefreshProvidersFunc
}
@@ -296,14 +277,6 @@ func New(ctx context.Context, logger slog.Logger, opts Options) (*Server, error)
allowedPorts = []string{"80", "443"}
}
// Build the boot-time router from DomainAllowlist + the lookup fn.
// Both empty is fine: the server fails closed (no MITM until
// Reload populates the router from the database).
bootRouter, err := buildBootRouter(opts.DomainAllowlist, opts.AIBridgeProviderFromHost, allowedPorts)
if err != nil {
return nil, err
}
// Parse configured exceptions to the blocked IP ranges.
allowedPrivateRanges := make([]net.IPNet, 0, len(opts.AllowedPrivateCIDRs))
for _, cidr := range opts.AllowedPrivateCIDRs {
@@ -352,13 +325,13 @@ func New(ctx context.Context, logger slog.Logger, opts Options) (*Server, error)
newDumper: opts.NewDumper,
metrics: opts.Metrics,
}
// Seed the boot-time router from the constructor inputs so the
// proxy can serve immediately. Reload may swap this snapshot at any
// point after construction.
srv.providerRouter.Store(bootRouter)
// Start with an empty router; the first Reload populates it from
// the configured provider source. The proxy fails closed (no MITM)
// until that happens.
srv.providerRouter.Store(emptyProviderRouter)
// Configure upstream proxy for tunneled (non-allowlisted) CONNECT requests.
// Allowlisted domains are MITM'd and forwarded to aibridge directly,
// Configure upstream proxy for tunneled (non-provider-host) CONNECT requests.
// Provider-host domains are MITM'd and forwarded to aibridge directly,
// bypassing the upstream proxy.
if opts.UpstreamProxy != "" {
upstreamURL, err := url.Parse(opts.UpstreamProxy)
@@ -443,7 +416,7 @@ func New(ctx context.Context, logger slog.Logger, opts Options) (*Server, error)
// Reject CONNECT requests to non-standard ports.
proxy.OnRequest().HandleConnectFunc(srv.portMiddleware(allowedPorts))
// Apply MITM with authentication only to allowlisted hosts. The host
// Apply MITM with authentication only to provider hosts. The host
// list is loaded from the atomic router on every CONNECT so a
// Reload while inflight requests are in progress takes effect on
// the next CONNECT without touching the already-MITM'd ones.
@@ -452,9 +425,9 @@ func New(ctx context.Context, logger slog.Logger, opts Options) (*Server, error)
srv.authMiddleware,
)
// Tunnel CONNECT requests for non-allowlisted domains directly to their destination.
// Tunnel CONNECT requests for non-provider-host domains directly to their destination.
// goproxy calls handlers in registration order: this must come after the MITM handler
// so it only handles requests that weren't matched by the allowlist.
// so it only handles requests that weren't matched as provider hosts.
proxy.OnRequest().HandleConnectFunc(srv.tunneledMiddleware)
// Handle decrypted requests: route to aibridged for known AI providers, or tunnel to original destination.
@@ -495,7 +468,6 @@ func New(ctx context.Context, logger slog.Logger, opts Options) (*Server, error)
slog.F("listen_addr", listener.Addr().String()),
slog.F("tls_listener_enabled", srv.tlsEnabled),
slog.F("coder_access_url", coderAccessURL.String()),
slog.F("domain_allowlist", bootRouter.mitmHosts),
slog.F("upstream_proxy", opts.UpstreamProxy),
slog.F("allowed_private_cidrs", opts.AllowedPrivateCIDRs),
slog.F("api_dump_enabled", opts.NewDumper != nil),
@@ -810,7 +782,7 @@ func newProxyAuthRequiredResponse(req *http.Request) *http.Response {
}
}
// tunneledMiddleware is a CONNECT middleware that handles tunneled (non-allowlisted)
// tunneledMiddleware is a CONNECT middleware that handles tunneled (non-provider-host)
// connections. These connections are not MITM'd and are tunneled directly to their
// destination. This middleware records metrics for tunneled CONNECT sessions.
func (s *Server) tunneledMiddleware(host string, _ *goproxy.ProxyCtx) (*goproxy.ConnectAction, string) {
@@ -946,16 +918,28 @@ func (s *Server) handleRequest(req *http.Request, ctx *goproxy.ProxyCtx) (*http.
return req, resp
}
if reqCtx.Provider == "" {
// A concurrent Reload can remove the provider after CONNECT
// authentication. The request is MITM'd (decrypted), but without a
// mapping there is no known route to aibridge. Log and forward
// to the original destination as a fallback.
s.logger.Warn(s.ctx, "decrypted request has no provider mapping, passing through",
// Re-validate the CONNECT-time provider against the live router.
// A long-lived CONNECT tunnel can outlive a provider being disabled,
// removed, or renamed: the captured reqCtx.Provider is stale, but
// subsequent decrypted requests would still route to aibridged if we
// trusted it. Look up the provider for the current request's host
// and pass through if the mapping is gone or has changed.
host := req.URL.Hostname()
if host == "" {
host = req.Host
if h, _, splitErr := net.SplitHostPort(host); splitErr == nil {
host = h
}
}
liveProvider := s.loadProviderRouter().providerFromHost(host)
if liveProvider == "" || liveProvider != reqCtx.Provider {
s.logger.Warn(s.ctx, "provider mapping changed or removed since CONNECT, passing through",
slog.F("connect_id", reqCtx.ConnectSessionID.String()),
slog.F("host", req.Host),
slog.F("method", req.Method),
slog.F("path", originalPath),
slog.F("connect_provider", reqCtx.Provider),
slog.F("live_provider", liveProvider),
)
return req, nil
}
@@ -1053,8 +1037,13 @@ func injectBYOKHeaderIfNeeded(header http.Header, coderToken string) {
}
// handleResponse handles responses received from aibridged.
// This is only called for MITM'd requests (allowlisted domains routed through aibridged).
// Tunneled requests (non-allowlisted domains) bypass this handler entirely.
// This is called for every MITM'd request, including the pass-through
// path where handleRequest re-validated the CONNECT-time provider and
// forwarded the request to the original upstream instead of aibridged.
// Pass-through responses are identified by reqCtx.RequestID == uuid.Nil
// (set only when handleRequest routes to aibridged) and are skipped here
// to avoid mislabeled logs and corrupting MITM metrics.
// Tunneled requests (non-provider-host domains) bypass this handler entirely.
func (s *Server) handleResponse(resp *http.Response, ctx *goproxy.ProxyCtx) *http.Response {
if resp == nil {
return nil
@@ -1077,6 +1066,14 @@ func (s *Server) handleResponse(resp *http.Response, ctx *goproxy.ProxyCtx) *htt
slog.F("status", resp.StatusCode),
)
// Pass-through responses (handleRequest returned without routing to
// aibridged) come from the real upstream. The aibridged-specific log
// and metrics do not apply; the pass-through itself is already logged
// in handleRequest.
if requestID == uuid.Nil {
return resp
}
switch {
case resp.StatusCode >= http.StatusInternalServerError:
logger.Error(s.ctx, "received error response from aibridged")
File diff suppressed because it is too large Load Diff
+66 -57
View File
@@ -3,7 +3,6 @@ package aibridgeproxyd
import (
"context"
"net/http"
"net/url"
"slices"
"strings"
@@ -13,23 +12,72 @@ import (
"cdr.dev/slog/v3"
)
// ProviderStatus describes the lifecycle state of a configured AI
// provider for observability and routing purposes.
type ProviderStatus string
const (
// ProviderStatusEnabled means the provider is configured, valid, and
// included in the active routing snapshot.
ProviderStatusEnabled ProviderStatus = "enabled"
// ProviderStatusDisabled means the provider exists in configuration
// but is intentionally turned off by an operator.
ProviderStatusDisabled ProviderStatus = "disabled"
// ProviderStatusError means the provider exists in configuration but
// cannot be routed to because of a validation failure (missing or
// invalid base URL, duplicate host, etc.).
ProviderStatusError ProviderStatus = "error"
)
// ReloadedProvider is one row from the provider configuration together
// with the outcome of evaluating it for routing. Host is populated only
// when Status == ProviderStatusEnabled; Err is populated only when
// Status == ProviderStatusError.
type ReloadedProvider struct {
Name string
Type string
Host string
Status ProviderStatus
Err error
}
// ProviderReload is the result of a single refresh pass: every
// configured provider with its classification.
type ProviderReload struct {
Providers []ReloadedProvider
}
// RefreshProvidersFunc returns the live provider classification used by
// Reload to rebuild the proxy's routing snapshot.
type RefreshProvidersFunc func(ctx context.Context) (ProviderReload, error)
// Reload refreshes proxy routing from the configured provider source.
// A refresh failure leaves the previous snapshot in place.
func (s *Server) Reload(ctx context.Context) error {
if s.refreshProviders == nil {
return nil
}
providers, err := s.refreshProviders(ctx)
reload, err := s.refreshProviders(ctx)
if err != nil {
return xerrors.Errorf("refresh ai providers for proxy routing: %w", err)
}
router, err := buildProviderRouter(ctx, s.logger, providers, s.allowedPorts)
router, err := buildProviderRouter(reload, s.allowedPorts)
if err != nil {
return xerrors.Errorf("build provider router (provider_count=%d): %w", len(providers), err)
return xerrors.Errorf("build provider router (provider_count=%d): %w", len(reload.Providers), err)
}
s.providerRouter.Store(router)
for _, p := range reload.Providers {
if p.Status == ProviderStatusError {
s.logger.Warn(s.ctx, "provider excluded from routing",
slog.F("provider", p.Name),
slog.Error(p.Err),
)
}
}
s.logger.Debug(s.ctx, "aibridgeproxyd router reloaded",
slog.F("provider_count", len(reload.Providers)),
slog.F("mitm_host_count", len(router.mitmHosts)),
slog.F("mitm_hosts", router.mitmHosts),
)
return nil
}
@@ -42,7 +90,7 @@ func (s *Server) loadProviderRouter() *providerRouter {
}
// mitmHostsCondition returns a goproxy ReqConditionFunc that reads the
// allowlist from the atomic router on every match. Using a closure
// MITM host set from the atomic router on every match. Using a closure
// instead of goproxy.ReqHostIs(...) lets Reload affect every later
// CONNECT without re-registering handlers.
func (s *Server) mitmHostsCondition() goproxy.ReqConditionFunc {
@@ -54,35 +102,23 @@ func (s *Server) mitmHostsCondition() goproxy.ReqConditionFunc {
}
}
// buildProviderRouter constructs a router snapshot from a refreshed
// provider list. First provider wins on duplicate hostnames.
func buildProviderRouter(ctx context.Context, logger slog.Logger, providers []ProviderRoute, allowedPorts []string) (*providerRouter, error) {
nameByHost := make(map[string]string, len(providers))
var domains []string
for _, p := range providers {
if p.BaseURL == "" {
logger.Warn(ctx, "skipping ai provider without base url",
slog.F("provider_name", p.Name),
)
// buildProviderRouter constructs a router snapshot from a classified
// provider reload. Only providers with Status == ProviderStatusEnabled
// are included in the active routing tables; the refresh function is
// responsible for classifying disabled and errored rows. First entry
// wins on duplicate hostnames as a defense-in-depth measure even though
// the refresh function should mark duplicates as errors.
func buildProviderRouter(reload ProviderReload, allowedPorts []string) (*providerRouter, error) {
nameByHost := make(map[string]string, len(reload.Providers))
domains := make([]string, 0, len(reload.Providers))
for _, p := range reload.Providers {
if p.Status != ProviderStatusEnabled {
continue
}
u, err := url.Parse(p.BaseURL)
if err != nil {
logger.Warn(ctx, "skipping ai provider with invalid base url",
slog.F("provider_name", p.Name),
slog.F("base_url", p.BaseURL),
slog.Error(err),
)
host := strings.ToLower(p.Host)
if host == "" {
continue
}
if u.Hostname() == "" {
logger.Warn(ctx, "skipping ai provider base url without hostname",
slog.F("provider_name", p.Name),
slog.F("base_url", p.BaseURL),
)
continue
}
host := strings.ToLower(u.Hostname())
if _, exists := nameByHost[host]; exists {
continue
}
@@ -95,30 +131,3 @@ func buildProviderRouter(ctx context.Context, logger slog.Logger, providers []Pr
}
return &providerRouter{mitmHosts: mitmHosts, nameByHost: nameByHost}, nil
}
// buildBootRouter seeds the providerRouter from the boot-time inputs.
// The lookup function is consulted only for hosts in the allowlist; a
// nil function with an empty allowlist is fine and yields an empty
// router (the proxy fails closed until Reload populates it).
func buildBootRouter(domainAllowlist []string, providerFromHost func(string) string, allowedPorts []string) (*providerRouter, error) {
mitmHosts, err := convertDomainsToHosts(domainAllowlist, allowedPorts)
if err != nil {
return nil, xerrors.Errorf("invalid domain allowlist: %w", err)
}
nameByHost := make(map[string]string, len(domainAllowlist))
for _, domain := range domainAllowlist {
domain = strings.TrimSpace(strings.ToLower(domain))
if domain == "" {
continue
}
var name string
if providerFromHost != nil {
name = providerFromHost(domain)
}
if name == "" {
return nil, xerrors.Errorf("domain %q is in allowlist but has no provider mapping", domain)
}
nameByHost[domain] = name
}
return &providerRouter{mitmHosts: mitmHosts, nameByHost: nameByHost}, nil
}
@@ -12,17 +12,26 @@ import (
"github.com/coder/coder/v2/testutil"
)
func enabledProvider(name, host string) ReloadedProvider {
return ReloadedProvider{
Name: name,
Type: "openai",
Host: host,
Status: ProviderStatusEnabled,
}
}
func TestServerReloadSwapsProviderRouter(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitShort)
providers := []ProviderRoute{{Name: "old", BaseURL: "https://old.example.com/"}}
reload := ProviderReload{Providers: []ReloadedProvider{enabledProvider("old", "old.example.com")}}
srv := &Server{
ctx: ctx,
logger: slogtest.Make(t, nil),
allowedPorts: []string{"443"},
refreshProviders: func(context.Context) ([]ProviderRoute, error) {
return providers, nil
refreshProviders: func(context.Context) (ProviderReload, error) {
return reload, nil
},
}
srv.providerRouter.Store(emptyProviderRouter)
@@ -31,7 +40,7 @@ func TestServerReloadSwapsProviderRouter(t *testing.T) {
assert.Equal(t, "old", srv.loadProviderRouter().providerFromHost("old.example.com"))
assert.Empty(t, srv.loadProviderRouter().providerFromHost("new.example.com"))
providers = []ProviderRoute{{Name: "new", BaseURL: "https://new.example.com/"}}
reload = ProviderReload{Providers: []ReloadedProvider{enabledProvider("new", "new.example.com")}}
require.NoError(t, srv.Reload(ctx))
router := srv.loadProviderRouter()
@@ -45,17 +54,17 @@ func TestServerReloadPreservesProviderRouterOnRefreshError(t *testing.T) {
ctx := testutil.Context(t, testutil.WaitShort)
refreshErr := xerrors.New("refresh failed")
providers := []ProviderRoute{{Name: "old", BaseURL: "https://old.example.com/"}}
reload := ProviderReload{Providers: []ReloadedProvider{enabledProvider("old", "old.example.com")}}
failRefresh := false
srv := &Server{
ctx: ctx,
logger: slogtest.Make(t, nil),
allowedPorts: []string{"443"},
refreshProviders: func(context.Context) ([]ProviderRoute, error) {
refreshProviders: func(context.Context) (ProviderReload, error) {
if failRefresh {
return nil, refreshErr
return ProviderReload{}, refreshErr
}
return providers, nil
return reload, nil
},
}
srv.providerRouter.Store(emptyProviderRouter)
@@ -73,75 +82,84 @@ func TestServerReloadPreservesProviderRouterOnRefreshError(t *testing.T) {
assert.Equal(t, []string{"old.example.com:443"}, after.mitmHosts)
}
// TestBuildProviderRouter covers the host-and-routing derivation that
// Reload feeds into the providerRouter.
// TestBuildProviderRouter covers the host-and-routing derivation from
// the classified provider reload.
func TestBuildProviderRouter(t *testing.T) {
t.Parallel()
t.Run("ExtractsHostnames", func(t *testing.T) {
t.Run("IncludesEnabledOnly", func(t *testing.T) {
t.Parallel()
providers := []ProviderRoute{
{Name: "openai", BaseURL: "https://api.openai.com/v1/"},
{Name: "anthropic", BaseURL: "https://api.anthropic.com/"},
{Name: "custom", BaseURL: "https://custom-llm.example.com:8443/api"},
}
reload := ProviderReload{Providers: []ReloadedProvider{
enabledProvider("openai", "api.openai.com"),
enabledProvider("anthropic", "api.anthropic.com"),
enabledProvider("custom", "custom-llm.example.com"),
// Host is populated on the non-enabled rows so the Status
// guard, not the empty-host guard, is what excludes them.
{Name: "off", Type: "openai", Host: "disabled.example.com", Status: ProviderStatusDisabled},
{Name: "bad", Type: "openai", Host: "errored.example.com", Status: ProviderStatusError, Err: xerrors.New("nope")},
}}
router, err := buildProviderRouter(testutil.Context(t, testutil.WaitShort), slogtest.Make(t, nil), providers, []string{"443"})
router, err := buildProviderRouter(reload, []string{"443"})
require.NoError(t, err)
assert.Equal(t, "openai", router.providerFromHost("api.openai.com"))
assert.Equal(t, "anthropic", router.providerFromHost("api.anthropic.com"))
assert.Equal(t, "custom", router.providerFromHost("custom-llm.example.com"))
assert.Empty(t, router.providerFromHost("unknown.com"))
assert.Empty(t, router.providerFromHost("disabled.example.com"),
"disabled provider must not be routable even with a populated Host")
assert.Empty(t, router.providerFromHost("errored.example.com"),
"errored provider must not be routable even with a populated Host")
assert.Contains(t, router.mitmHosts, "api.openai.com:443")
assert.Contains(t, router.mitmHosts, "api.anthropic.com:443")
})
t.Run("DeduplicatesSameHost", func(t *testing.T) {
t.Parallel()
providers := []ProviderRoute{
{Name: "first", BaseURL: "https://api.example.com/v1"},
{Name: "second", BaseURL: "https://api.example.com/v2"},
}
router, err := buildProviderRouter(testutil.Context(t, testutil.WaitShort), slogtest.Make(t, nil), providers, []string{"443"})
require.NoError(t, err)
// First provider wins on duplicate host.
assert.Equal(t, "first", router.providerFromHost("api.example.com"))
assert.Len(t, router.mitmHosts, 3)
})
t.Run("CaseInsensitive", func(t *testing.T) {
t.Parallel()
providers := []ProviderRoute{
{Name: "provider", BaseURL: "https://API.Example.COM/v1"},
}
reload := ProviderReload{Providers: []ReloadedProvider{
{Name: "provider", Type: "openai", Host: "API.Example.COM", Status: ProviderStatusEnabled},
}}
router, err := buildProviderRouter(testutil.Context(t, testutil.WaitShort), slogtest.Make(t, nil), providers, []string{"443"})
router, err := buildProviderRouter(reload, []string{"443"})
require.NoError(t, err)
assert.Equal(t, "provider", router.providerFromHost("API.Example.COM"))
assert.Equal(t, "provider", router.providerFromHost("api.example.com"))
})
t.Run("SkipsEmptyOrMalformedBaseURL", func(t *testing.T) {
t.Run("DefensiveDeduplicatesSameHost", func(t *testing.T) {
t.Parallel()
providers := []ProviderRoute{
{Name: "no-url"},
{Name: "scheme-only", BaseURL: "https://"},
{Name: "good", BaseURL: "https://api.good.example.com/"},
}
// Refresh function should mark the duplicate as ProviderStatusError;
// buildProviderRouter is defensive and tolerates an enabled duplicate
// by giving the first entry the host (first wins).
reload := ProviderReload{Providers: []ReloadedProvider{
enabledProvider("first", "api.example.com"),
enabledProvider("second", "api.example.com"),
}}
router, err := buildProviderRouter(testutil.Context(t, testutil.WaitShort), slogtest.Make(t, nil), providers, []string{"443"})
router, err := buildProviderRouter(reload, []string{"443"})
require.NoError(t, err)
assert.Equal(t, "first", router.providerFromHost("api.example.com"))
})
t.Run("SkipsRowsWithEmptyHost", func(t *testing.T) {
t.Parallel()
reload := ProviderReload{Providers: []ReloadedProvider{
{Name: "no-host", Type: "openai", Status: ProviderStatusEnabled},
enabledProvider("good", "api.good.example.com"),
}}
router, err := buildProviderRouter(reload, []string{"443"})
require.NoError(t, err)
assert.Equal(t, "good", router.providerFromHost("api.good.example.com"))
assert.Empty(t, router.providerFromHost("scheme-only"))
assert.Equal(t, []string{"api.good.example.com:443"}, router.mitmHosts)
})
}
+222 -66
View File
@@ -5,6 +5,7 @@ import (
"io"
"net/http"
"net/http/httptest"
"net/url"
"slices"
"strings"
"sync"
@@ -55,16 +56,24 @@ func (r *aibridgedRecorder) reset() {
r.path = ""
}
// providerStore is a mutable [aibridgeproxyd.RefreshProvidersFunc]
// backing for integration tests. set / setErr mutate the snapshot
// returned by the next Reload, mimicking CRUD against the database.
// rawProvider is a (name, base URL) pair representing what the database
// holds before classification, mirroring the ai_providers row shape
// that the production refresh function classifies.
type rawProvider struct {
name string
baseURL string
}
// providerStore is a mutable RefreshProvidersFunc backing for
// integration tests. set / setErr mutate the snapshot returned by the
// next Reload, mimicking CRUD against the database.
type providerStore struct {
mu sync.Mutex
providers []aibridgeproxyd.ProviderRoute
providers []rawProvider
err error
}
func (s *providerStore) set(providers []aibridgeproxyd.ProviderRoute) {
func (s *providerStore) set(providers []rawProvider) {
s.mu.Lock()
defer s.mu.Unlock()
s.providers = providers
@@ -77,20 +86,59 @@ func (s *providerStore) setErr(err error) {
s.err = err
}
func (s *providerStore) refresh(context.Context) ([]aibridgeproxyd.ProviderRoute, error) {
func (s *providerStore) refresh(context.Context) (aibridgeproxyd.ProviderReload, error) {
s.mu.Lock()
defer s.mu.Unlock()
if s.err != nil {
return nil, s.err
return aibridgeproxyd.ProviderReload{}, s.err
}
// Return a copy so callers can't mutate our internal snapshot.
return slices.Clone(s.providers), nil
providers := slices.Clone(s.providers)
reload := aibridgeproxyd.ProviderReload{
Providers: make([]aibridgeproxyd.ReloadedProvider, 0, len(providers)),
}
seenHost := make(map[string]string, len(providers))
for _, p := range providers {
reload.Providers = append(reload.Providers, classifyRaw(p, seenHost))
}
return reload, nil
}
// newReloadTestHarness boots a proxy with an empty boot allowlist and a
// store-backed RefreshProviders. Production wiring is identical: the
// daemon constructs the proxy without a static allowlist and lets
// Reload populate the router from the database.
// classifyRaw mirrors the production classifier in enterprise/cli so
// the reload tests exercise the same validation rules end-to-end.
func classifyRaw(p rawProvider, seenHost map[string]string) aibridgeproxyd.ReloadedProvider {
out := aibridgeproxyd.ReloadedProvider{Name: p.name, Type: "openai"}
if strings.TrimSpace(p.baseURL) == "" {
out.Status = aibridgeproxyd.ProviderStatusError
out.Err = xerrors.New("base url is empty")
return out
}
u, err := url.Parse(p.baseURL)
if err != nil {
out.Status = aibridgeproxyd.ProviderStatusError
out.Err = xerrors.Errorf("invalid base url %q: %w", p.baseURL, err)
return out
}
host := strings.ToLower(u.Hostname())
if host == "" {
out.Status = aibridgeproxyd.ProviderStatusError
out.Err = xerrors.Errorf("base url %q has no hostname", p.baseURL)
return out
}
if claimedBy, taken := seenHost[host]; taken {
out.Status = aibridgeproxyd.ProviderStatusError
out.Err = xerrors.Errorf("hostname %q already claimed by provider %q", host, claimedBy)
return out
}
seenHost[host] = p.name
out.Host = host
out.Status = aibridgeproxyd.ProviderStatusEnabled
return out
}
// newReloadTestHarness boots a proxy with an empty initial router and
// a store-backed RefreshProviders. Production wiring is identical: the
// daemon constructs the proxy without preconfigured provider hosts and
// lets Reload populate the router from the database.
func newReloadTestHarness(t *testing.T) *reloadTestHarness {
t.Helper()
@@ -106,10 +154,6 @@ func newReloadTestHarness(t *testing.T) *reloadTestHarness {
srv := newTestProxy(t,
withCoderAccessURL(bridged.URL),
withAllowedPorts("443"),
// Empty boot allowlist: the router must be populated by Reload,
// matching the production daemon's behavior.
withDomainAllowlist(),
withAIBridgeProviderFromHost(nil),
withRefreshProviders(store.refresh),
)
@@ -192,16 +236,128 @@ func (h *reloadTestHarness) expectNotRouted(t *testing.T, targetURL string) {
"aibridged must not be reached for non-routed host %s", targetURL)
}
// TestProxy_StaleTunnelStopsRoutingAfterProviderChange is the
// regression test for a bug where a long-lived CONNECT tunnel that was
// established while a provider was enabled kept routing decrypted
// requests to aibridged after the provider was disabled or renamed. The
// fix re-validates the CONNECT-time provider against the live router on
// every decrypted request and covers both shapes of stale mapping:
//
// - ProviderDisabled: liveProvider == "" (host no longer MITM'd).
// - ProviderRenamed: liveProvider != reqCtx.Provider (host MITM'd, but
// under a new provider name).
func TestProxy_StaleTunnelStopsRoutingAfterProviderChange(t *testing.T) {
t.Parallel()
testCases := []struct {
name string
// applyChange mutates the store to simulate the provider change
// after the initial routed request succeeds.
applyChange func(*providerStore)
// changeDescription is appended to the second-request assertion
// message so a failure points at the exercised branch.
changeDescription string
}{
{
name: "ProviderDisabled",
applyChange: func(s *providerStore) { s.set(nil) },
changeDescription: "after alpha was disabled",
},
{
name: "ProviderRenamed",
applyChange: func(s *providerStore) {
// Same host, new provider name: the live router still
// MITMs alpha.invalid, but as "alpha-v2". The stale
// CONNECT-time name "alpha" no longer matches.
s.set([]rawProvider{
{name: "alpha-v2", baseURL: "https://alpha.invalid/v1"},
})
},
changeDescription: "after alpha was renamed to alpha-v2",
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
recorder := &aibridgedRecorder{}
bridged := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
recorder.record(r.URL.Path)
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte("aibridged"))
}))
t.Cleanup(bridged.Close)
store := &providerStore{}
store.set([]rawProvider{
{name: "alpha", baseURL: "https://alpha.invalid/v1"},
})
// newTestProxy seeds the router from the store via the
// initial Reload, so the first CONNECT is MITM'd as alpha.
srv := newTestProxy(t,
withCoderAccessURL(bridged.URL),
withAllowedPorts("443"),
withRefreshProviders(store.refresh),
)
certPool := getProxyCertPool(t)
client := newProxyClient(t, srv, makeProxyAuthHeader("coder-token"), certPool, false)
// Keep-alives are required: the regression exists only when a
// subsequent request reuses the original CONNECT tunnel. A fresh
// CONNECT would correctly observe the post-reload router.
transport := client.Transport.(*http.Transport)
transport.DisableKeepAlives = false
transport.MaxConnsPerHost = 1
transport.MaxIdleConnsPerHost = 1
sendThroughTunnel := func(path string) (status int, err error) {
ctx, cancel := context.WithTimeout(t.Context(), testutil.WaitShort)
defer cancel()
req, reqErr := http.NewRequestWithContext(ctx, http.MethodPost, "https://alpha.invalid"+path, strings.NewReader(`{}`))
require.NoError(t, reqErr)
req.Header.Set("Content-Type", "application/json")
resp, err := client.Do(req)
if err != nil {
return 0, err
}
defer resp.Body.Close()
_, _ = io.Copy(io.Discard, resp.Body)
return resp.StatusCode, nil
}
// First request: alpha is enabled, the proxy MITMs and routes to
// aibridged under the alpha namespace.
recorder.reset()
status, err := sendThroughTunnel("/v1/messages")
require.NoError(t, err)
require.Equal(t, http.StatusOK, status)
require.Equal(t, "/api/v2/aibridge/alpha/v1/messages", recorder.load(),
"first request must be routed to aibridged while alpha is enabled")
// Apply the provider change and reload. The atomic router swap
// takes effect immediately, but the client's connection (and
// the proxy's hijacked tunnel) remain open.
tc.applyChange(store)
require.NoError(t, srv.Reload(t.Context()))
// Second request on the same tunnel: aibridged must NOT see it.
// The connection is hijacked so the request reaches the proxy's
// handleRequest with the stale CONNECT-time provider; the fix
// re-validates against the live router and passes through to
// the original upstream (alpha.invalid, which fails DNS).
recorder.reset()
_, _ = sendThroughTunnel("/v1/should-not-route")
require.Empty(t, recorder.load(),
"%s, aibridged must not receive the request even on a reused tunnel", tc.changeDescription)
})
}
}
// TestProxy_HotReloadRoutingCRUD drives the proxy through a CRUD-style
// sequence of provider changes and asserts on routing after each
// Reload via real HTTPS requests. Each sub-test mutates the store and
// validates that:
// - newly created providers are MITM'd to aibridged with the right
// /api/v2/aibridge/<name>/<path>
// - renamed providers route under the new name
// - providers whose BaseURL host changes route the new host and stop
// MITM'ing the old host
// - deleted providers stop being MITM'd; aibridged sees nothing
// Reload via real HTTPS requests.
//
// Hostnames are .invalid (RFC 2606) so a request that escapes the MITM
// path fails fast via DNS rather than reaching a real upstream.
@@ -210,30 +366,30 @@ func TestProxy_HotReloadRoutingCRUD(t *testing.T) {
h := newReloadTestHarness(t)
// InitialEmptyRouter: no Reload has been called and the boot
// allowlist is empty, so any host falls through to the tunneled
// InitialEmptyRouter: no Reload has been called and no provider
// hosts are configured, so any host falls through to the tunneled
// middleware.
h.expectNotRouted(t, "https://alpha.invalid/v1/messages")
// CreateProvider.
h.store.set([]aibridgeproxyd.ProviderRoute{
{Name: "alpha", BaseURL: "https://alpha.invalid/v1"},
h.store.set([]rawProvider{
{name: "alpha", baseURL: "https://alpha.invalid/v1"},
})
require.NoError(t, h.srv.Reload(t.Context()))
h.expectRoutedTo(t, "https://alpha.invalid/v1/messages", "/api/v2/aibridge/alpha/v1/messages")
// UpdateProviderName: the same BaseURL with a new name must route
// under the new name on the next Reload.
h.store.set([]aibridgeproxyd.ProviderRoute{
{Name: "alpha-v2", BaseURL: "https://alpha.invalid/v1"},
h.store.set([]rawProvider{
{name: "alpha-v2", baseURL: "https://alpha.invalid/v1"},
})
require.NoError(t, h.srv.Reload(t.Context()))
h.expectRoutedTo(t, "https://alpha.invalid/v1/messages", "/api/v2/aibridge/alpha-v2/v1/messages")
// UpdateProviderBaseURLHost: moving the provider to a new host must
// start MITM'ing the new host and stop MITM'ing the old one.
h.store.set([]aibridgeproxyd.ProviderRoute{
{Name: "alpha-v2", BaseURL: "https://alpha-new.invalid/v1"},
h.store.set([]rawProvider{
{name: "alpha-v2", baseURL: "https://alpha-new.invalid/v1"},
})
require.NoError(t, h.srv.Reload(t.Context()))
h.expectRoutedTo(t, "https://alpha-new.invalid/v1/messages", "/api/v2/aibridge/alpha-v2/v1/messages")
@@ -241,9 +397,9 @@ func TestProxy_HotReloadRoutingCRUD(t *testing.T) {
// AddSecondProvider: a second provider added in the same Reload must
// route independently from the first.
h.store.set([]aibridgeproxyd.ProviderRoute{
{Name: "alpha-v2", BaseURL: "https://alpha-new.invalid/v1"},
{Name: "beta", BaseURL: "https://beta.invalid/v1"},
h.store.set([]rawProvider{
{name: "alpha-v2", baseURL: "https://alpha-new.invalid/v1"},
{name: "beta", baseURL: "https://beta.invalid/v1"},
})
require.NoError(t, h.srv.Reload(t.Context()))
h.expectRoutedTo(t, "https://alpha-new.invalid/v1/messages", "/api/v2/aibridge/alpha-v2/v1/messages")
@@ -251,8 +407,8 @@ func TestProxy_HotReloadRoutingCRUD(t *testing.T) {
// DeleteOneProvider: removing alpha must keep beta routed and stop
// routing alpha.
h.store.set([]aibridgeproxyd.ProviderRoute{
{Name: "beta", BaseURL: "https://beta.invalid/v1"},
h.store.set([]rawProvider{
{name: "beta", baseURL: "https://beta.invalid/v1"},
})
require.NoError(t, h.srv.Reload(t.Context()))
h.expectRoutedTo(t, "https://beta.invalid/v1/chat/completions", "/api/v2/aibridge/beta/v1/chat/completions")
@@ -268,8 +424,8 @@ func TestProxy_HotReloadRoutingCRUD(t *testing.T) {
// RecreateAfterDelete: reintroducing a previously-deleted provider
// must route again without restart, confirming the swap is
// symmetric.
h.store.set([]aibridgeproxyd.ProviderRoute{
{Name: "alpha", BaseURL: "https://alpha.invalid/v1"},
h.store.set([]rawProvider{
{name: "alpha", baseURL: "https://alpha.invalid/v1"},
})
require.NoError(t, h.srv.Reload(t.Context()))
h.expectRoutedTo(t, "https://alpha.invalid/v1/messages", "/api/v2/aibridge/alpha/v1/messages")
@@ -288,11 +444,11 @@ func TestProxy_HotReloadRoutingInvalidProviders(t *testing.T) {
h := newReloadTestHarness(t)
// One valid provider and one with an empty BaseURL. The empty
// entry must be silently dropped; the valid one must still
// route.
h.store.set([]aibridgeproxyd.ProviderRoute{
{Name: "no-url"},
{Name: "valid", BaseURL: "https://valid.invalid/v1"},
// entry must be classified as error and excluded from routing;
// the valid one must still route.
h.store.set([]rawProvider{
{name: "no-url"},
{name: "valid", baseURL: "https://valid.invalid/v1"},
})
require.NoError(t, h.srv.Reload(t.Context()))
@@ -304,12 +460,12 @@ func TestProxy_HotReloadRoutingInvalidProviders(t *testing.T) {
h := newReloadTestHarness(t)
// A BaseURL that fails url.Parse and one whose Hostname() is
// empty must both be dropped. Mixed with a valid entry, only
// the valid one routes.
h.store.set([]aibridgeproxyd.ProviderRoute{
{Name: "malformed", BaseURL: "://not-a-url"},
{Name: "no-host", BaseURL: "https://"},
{Name: "valid", BaseURL: "https://valid.invalid/v1"},
// empty must both be classified as error. Mixed with a valid
// entry, only the valid one routes.
h.store.set([]rawProvider{
{name: "malformed", baseURL: "://not-a-url"},
{name: "no-host", baseURL: "https://"},
{name: "valid", baseURL: "https://valid.invalid/v1"},
})
require.NoError(t, h.srv.Reload(t.Context()))
@@ -320,11 +476,11 @@ func TestProxy_HotReloadRoutingInvalidProviders(t *testing.T) {
t.Parallel()
h := newReloadTestHarness(t)
// Two providers with the same BaseURL host: the first one wins,
// matching buildProviderRouter's documented contract.
h.store.set([]aibridgeproxyd.ProviderRoute{
{Name: "first", BaseURL: "https://shared.invalid/v1"},
{Name: "second", BaseURL: "https://shared.invalid/v2"},
// Two providers with the same BaseURL host: the second is
// classified as error and excluded; the first routes.
h.store.set([]rawProvider{
{name: "first", baseURL: "https://shared.invalid/v1"},
{name: "second", baseURL: "https://shared.invalid/v2"},
})
require.NoError(t, h.srv.Reload(t.Context()))
@@ -337,10 +493,10 @@ func TestProxy_HotReloadRoutingInvalidProviders(t *testing.T) {
h := newReloadTestHarness(t)
// When every provider is invalid, the router contains no
// entries and the proxy fails closed: no host is MITM'd.
h.store.set([]aibridgeproxyd.ProviderRoute{
{Name: "no-url"},
{Name: "malformed", BaseURL: "://not-a-url"},
{Name: "no-host", BaseURL: "https://"},
h.store.set([]rawProvider{
{name: "no-url"},
{name: "malformed", baseURL: "://not-a-url"},
{name: "no-host", baseURL: "https://"},
})
require.NoError(t, h.srv.Reload(t.Context()))
@@ -352,15 +508,15 @@ func TestProxy_HotReloadRoutingInvalidProviders(t *testing.T) {
h := newReloadTestHarness(t)
// Seed a valid snapshot so we have something to preserve.
h.store.set([]aibridgeproxyd.ProviderRoute{
{Name: "alpha", BaseURL: "https://alpha.invalid/v1"},
h.store.set([]rawProvider{
{name: "alpha", baseURL: "https://alpha.invalid/v1"},
})
require.NoError(t, h.srv.Reload(t.Context()))
h.expectRoutedTo(t, "https://alpha.invalid/v1/messages", "/api/v2/aibridge/alpha/v1/messages")
// A refresh error must NOT clear the router: dropping the
// allowlist on every transient DB hiccup would amplify the
// fault into a denial of service.
// provider host set on every transient DB hiccup would
// amplify the fault into a denial of service.
h.store.setErr(xerrors.New("simulated db failure"))
err := h.srv.Reload(t.Context())
require.Error(t, err)
@@ -369,8 +525,8 @@ func TestProxy_HotReloadRoutingInvalidProviders(t *testing.T) {
// Recovery: once the store returns providers again, the next
// Reload applies the new snapshot.
h.store.set([]aibridgeproxyd.ProviderRoute{
{Name: "beta", BaseURL: "https://beta.invalid/v1"},
h.store.set([]rawProvider{
{name: "beta", baseURL: "https://beta.invalid/v1"},
})
require.NoError(t, h.srv.Reload(t.Context()))
h.expectRoutedTo(t, "https://beta.invalid/v1/messages", "/api/v2/aibridge/beta/v1/messages")
+56 -6
View File
@@ -5,7 +5,9 @@ package cli
import (
"context"
"io"
"net/url"
"path/filepath"
"strings"
"github.com/prometheus/client_golang/prometheus"
"golang.org/x/xerrors"
@@ -86,19 +88,67 @@ func newAIBridgeProxyDaemon(coderAPI *coderd.API) (io.Closer, error) {
}, nil
}
// refreshProxyProviders classifies every ai_providers row as enabled,
// disabled, or error so the proxy router and any observers see the full
// configured set. Disabled rows are excluded from routing; errored rows
// are excluded from routing and surface their failure reason for
// metrics and logs.
func refreshProxyProviders(db database.Store) aibridgeproxyd.RefreshProvidersFunc {
return func(ctx context.Context) ([]aibridgeproxyd.ProviderRoute, error) {
return func(ctx context.Context) (aibridgeproxyd.ProviderReload, error) {
//nolint:gocritic // AsAIProviderMetadataReader is the correct subject for routing-only access.
rows, err := db.GetAIProviders(dbauthz.AsAIProviderMetadataReader(ctx), database.GetAIProvidersParams{
IncludeDisabled: false,
IncludeDisabled: true,
})
if err != nil {
return nil, xerrors.Errorf("load ai providers: %w", err)
return aibridgeproxyd.ProviderReload{}, xerrors.Errorf("load ai providers: %w", err)
}
out := make([]aibridgeproxyd.ProviderRoute, 0, len(rows))
reload := aibridgeproxyd.ProviderReload{
Providers: make([]aibridgeproxyd.ReloadedProvider, 0, len(rows)),
}
seenHost := make(map[string]string, len(rows))
for _, row := range rows {
out = append(out, aibridgeproxyd.ProviderRoute{Name: row.Name, BaseURL: row.BaseUrl})
reload.Providers = append(reload.Providers, classifyProviderRow(row, seenHost))
}
return out, nil
return reload, nil
}
}
// classifyProviderRow evaluates a single ai_providers row for routing.
// seenHost is mutated to track the first provider that claimed each
// hostname so later duplicates can be flagged as errors.
func classifyProviderRow(row database.AIProvider, seenHost map[string]string) aibridgeproxyd.ReloadedProvider {
out := aibridgeproxyd.ReloadedProvider{
Name: row.Name,
Type: string(row.Type),
}
if !row.Enabled {
out.Status = aibridgeproxyd.ProviderStatusDisabled
return out
}
if strings.TrimSpace(row.BaseUrl) == "" {
out.Status = aibridgeproxyd.ProviderStatusError
out.Err = xerrors.New("base url is empty")
return out
}
u, err := url.Parse(row.BaseUrl)
if err != nil {
out.Status = aibridgeproxyd.ProviderStatusError
out.Err = xerrors.Errorf("invalid base url %q: %w", row.BaseUrl, err)
return out
}
host := strings.ToLower(u.Hostname())
if host == "" {
out.Status = aibridgeproxyd.ProviderStatusError
out.Err = xerrors.Errorf("base url %q has no hostname", row.BaseUrl)
return out
}
if claimedBy, taken := seenHost[host]; taken {
out.Status = aibridgeproxyd.ProviderStatusError
out.Err = xerrors.Errorf("hostname %q already claimed by provider %q", host, claimedBy)
return out
}
seenHost[host] = row.Name
out.Host = host
out.Status = aibridgeproxyd.ProviderStatusEnabled
return out
}
@@ -0,0 +1,105 @@
//go:build !slim
package cli
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/enterprise/aibridgeproxyd"
)
// TestClassifyProviderRow covers every branch of the classifier so the
// disabled, error, and enabled paths are exercised through the
// production code instead of relying on classifyRaw, the test mirror in
// reload_test.go.
func TestClassifyProviderRow(t *testing.T) {
t.Parallel()
enabledRow := func(name, baseURL string) database.AIProvider {
return database.AIProvider{
Name: name,
Type: database.AiProviderTypeOpenai,
Enabled: true,
BaseUrl: baseURL,
}
}
t.Run("Enabled", func(t *testing.T) {
t.Parallel()
seen := map[string]string{}
got := classifyProviderRow(enabledRow("openai", "https://api.openai.com/v1"), seen)
assert.Equal(t, "openai", got.Name)
assert.Equal(t, string(database.AiProviderTypeOpenai), got.Type)
assert.Equal(t, aibridgeproxyd.ProviderStatusEnabled, got.Status)
assert.Equal(t, "api.openai.com", got.Host)
assert.NoError(t, got.Err)
assert.Equal(t, "openai", seen["api.openai.com"])
})
t.Run("DisabledRow", func(t *testing.T) {
t.Parallel()
seen := map[string]string{}
row := enabledRow("off", "https://api.off.example.com/v1")
row.Enabled = false
got := classifyProviderRow(row, seen)
assert.Equal(t, aibridgeproxyd.ProviderStatusDisabled, got.Status)
assert.Empty(t, got.Host, "disabled provider must not claim a host")
assert.NoError(t, got.Err)
assert.Empty(t, seen, "disabled provider must not occupy a host slot")
})
t.Run("EmptyBaseURL", func(t *testing.T) {
t.Parallel()
seen := map[string]string{}
got := classifyProviderRow(enabledRow("no-url", " "), seen)
assert.Equal(t, aibridgeproxyd.ProviderStatusError, got.Status)
assert.Empty(t, got.Host)
assert.ErrorContains(t, got.Err, "base url is empty")
})
t.Run("MalformedBaseURL", func(t *testing.T) {
t.Parallel()
seen := map[string]string{}
got := classifyProviderRow(enabledRow("bad", "://not-a-url"), seen)
assert.Equal(t, aibridgeproxyd.ProviderStatusError, got.Status)
assert.ErrorContains(t, got.Err, "invalid base url")
})
t.Run("BaseURLWithoutHostname", func(t *testing.T) {
t.Parallel()
seen := map[string]string{}
got := classifyProviderRow(enabledRow("no-host", "https://"), seen)
assert.Equal(t, aibridgeproxyd.ProviderStatusError, got.Status)
assert.ErrorContains(t, got.Err, "no hostname")
})
t.Run("DuplicateHostnameFirstWins", func(t *testing.T) {
t.Parallel()
seen := map[string]string{}
first := classifyProviderRow(enabledRow("first", "https://shared.example.com/v1"), seen)
assert.Equal(t, aibridgeproxyd.ProviderStatusEnabled, first.Status)
second := classifyProviderRow(enabledRow("second", "https://shared.example.com/v2"), seen)
assert.Equal(t, aibridgeproxyd.ProviderStatusError, second.Status)
assert.ErrorContains(t, second.Err, "already claimed by provider \"first\"")
assert.Equal(t, "first", seen["shared.example.com"], "first wins must not be overwritten")
})
t.Run("HostnameLowercased", func(t *testing.T) {
t.Parallel()
seen := map[string]string{}
got := classifyProviderRow(enabledRow("mixed", "https://API.Example.COM/v1"), seen)
assert.Equal(t, aibridgeproxyd.ProviderStatusEnabled, got.Status)
assert.Equal(t, "api.example.com", got.Host)
})
}