Skip to content

Commit a388093

Browse files
Registry policy gates with type-safe configs (#4711)
Separate runner and registry policy gates with type-safe configs The previous commit extracted PolicyGate to a shared pkg/policy package, but this mixed runner and registry concerns in a single interface and forced CheckCreateServer to use `any` to avoid a circular dependency. Instead, keep each policy gate in the package it guards: - pkg/runner keeps its original PolicyGate with type-safe *RunConfig - pkg/registry gets a new PolicyGate with CheckUpdateRegistry and CheckDeleteRegistry, each receiving a dedicated config struct (UpdateRegistryConfig, DeleteRegistryConfig) that both CLI and API callers populate with the information available at their call site The registry gate uses sync.RWMutex (read-heavy, write-once pattern), NoopPolicyGate as the default (no separate allowAllGate), and exports SnapshotPolicyGate for test cleanup across packages. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 67bfd98 commit a388093

9 files changed

Lines changed: 466 additions & 4 deletions

File tree

cmd/thv/app/config.go

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"errors"
88
"fmt"
99
"os"
10+
"strings"
1011

1112
"github.com/spf13/cobra"
1213

@@ -180,6 +181,19 @@ func unsetCACertCmdFunc(_ *cobra.Command, _ []string) error {
180181
func setRegistryCmdFunc(cmd *cobra.Command, args []string) error {
181182
input := args[0]
182183

184+
cfg := &registry.UpdateRegistryConfig{
185+
AllowPrivateIP: allowPrivateRegistryIp,
186+
HasAuth: registryAuthIssuer != "" && registryAuthClientID != "",
187+
}
188+
if strings.HasPrefix(input, "http://") || strings.HasPrefix(input, "https://") {
189+
cfg.URL = input
190+
} else {
191+
cfg.LocalPath = input
192+
}
193+
if err := registry.ActivePolicyGate().CheckUpdateRegistry(cmd.Context(), cfg); err != nil {
194+
return err
195+
}
196+
183197
// Always clear existing auth when changing registry (security: prevents
184198
// tokens from being sent to the wrong server).
185199
provider := config.NewDefaultProvider()
@@ -236,7 +250,13 @@ func getRegistryCmdFunc(_ *cobra.Command, _ []string) error {
236250
return nil
237251
}
238252

239-
func unsetRegistryCmdFunc(_ *cobra.Command, _ []string) error {
253+
func unsetRegistryCmdFunc(cmd *cobra.Command, _ []string) error {
254+
if err := registry.ActivePolicyGate().CheckDeleteRegistry(cmd.Context(), &registry.DeleteRegistryConfig{
255+
Name: "default",
256+
}); err != nil {
257+
return err
258+
}
259+
240260
service := registry.NewConfigurator()
241261
err := service.UnsetRegistry()
242262
if err != nil {

docs/server/docs.go

Lines changed: 20 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/server/swagger.json

Lines changed: 20 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/server/swagger.yaml

Lines changed: 12 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/api/v1/registry.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,7 @@ func (rr *RegistryRoutes) getRegistry(w http.ResponseWriter, r *http.Request) {
494494
// @Param body body UpdateRegistryRequest true "Registry configuration"
495495
// @Success 200 {object} UpdateRegistryResponse
496496
// @Failure 400 {string} string "Bad Request"
497+
// @Failure 403 {string} string "Forbidden - blocked by policy"
497498
// @Failure 404 {string} string "Not Found"
498499
// @Failure 502 {string} string "Bad Gateway - Registry validation failed"
499500
// @Failure 504 {string} string "Gateway Timeout - Registry unreachable"
@@ -519,6 +520,11 @@ func (rr *RegistryRoutes) updateRegistry(w http.ResponseWriter, r *http.Request)
519520
return
520521
}
521522

523+
if err := regpkg.ActivePolicyGate().CheckUpdateRegistry(r.Context(), updateRegistryConfigFromRequest(&req)); err != nil {
524+
http.Error(w, err.Error(), http.StatusForbidden)
525+
return
526+
}
527+
522528
// Process the registry URL/path update.
523529
var responseType string
524530
registryType, err := rr.processRegistryUpdate(&req)
@@ -588,6 +594,27 @@ func validateRegistryRequest(req *UpdateRegistryRequest) error {
588594
return nil
589595
}
590596

597+
// updateRegistryConfigFromRequest builds an UpdateRegistryConfig from the
598+
// parsed API request for policy evaluation.
599+
func updateRegistryConfigFromRequest(req *UpdateRegistryRequest) *regpkg.UpdateRegistryConfig {
600+
cfg := &regpkg.UpdateRegistryConfig{
601+
HasAuth: req.Auth != nil,
602+
}
603+
if req.URL != nil {
604+
cfg.URL = *req.URL
605+
}
606+
if req.APIURL != nil {
607+
cfg.APIURL = *req.APIURL
608+
}
609+
if req.LocalPath != nil {
610+
cfg.LocalPath = *req.LocalPath
611+
}
612+
if req.AllowPrivateIP != nil {
613+
cfg.AllowPrivateIP = *req.AllowPrivateIP
614+
}
615+
return cfg
616+
}
617+
591618
// processAuthUpdate validates and applies OAuth configuration for registry auth.
592619
func (rr *RegistryRoutes) processAuthUpdate(ctx context.Context, authReq *UpdateRegistryAuthRequest) error {
593620
if authReq.Issuer == "" || authReq.ClientID == "" {
@@ -654,11 +681,19 @@ func (rr *RegistryRoutes) processRegistryUpdate(req *UpdateRegistryRequest) (str
654681
// @Produce json
655682
// @Param name path string true "Registry name"
656683
// @Success 204 {string} string "No Content"
684+
// @Failure 403 {string} string "Forbidden - blocked by policy"
657685
// @Failure 404 {string} string "Not Found"
658686
// @Router /api/v1beta/registry/{name} [delete]
659687
func (*RegistryRoutes) removeRegistry(w http.ResponseWriter, r *http.Request) {
660688
name := chi.URLParam(r, "name")
661689

690+
if err := regpkg.ActivePolicyGate().CheckDeleteRegistry(r.Context(), &regpkg.DeleteRegistryConfig{
691+
Name: name,
692+
}); err != nil {
693+
http.Error(w, err.Error(), http.StatusForbidden)
694+
return
695+
}
696+
662697
// Cannot remove the default registry
663698
if name == defaultRegistryName {
664699
http.Error(w, "Cannot remove the default registry", http.StatusBadRequest)

pkg/api/v1/registry_test.go

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package v1
66
import (
77
"context"
88
"encoding/json"
9+
"errors"
910
"net/http"
1011
"net/http/httptest"
1112
"os"
@@ -345,3 +346,92 @@ func TestRegistryAPI_PutEndpoint(t *testing.T) {
345346
})
346347
}
347348
}
349+
350+
// denyRegistryGate is a test helper that blocks all registry mutations.
351+
type denyRegistryGate struct {
352+
registry.NoopPolicyGate
353+
err error
354+
}
355+
356+
func (g *denyRegistryGate) CheckUpdateRegistry(_ context.Context, _ *registry.UpdateRegistryConfig) error {
357+
return g.err
358+
}
359+
360+
func (g *denyRegistryGate) CheckDeleteRegistry(_ context.Context, _ *registry.DeleteRegistryConfig) error {
361+
return g.err
362+
}
363+
364+
//nolint:paralleltest // Mutates global registry policy gate singleton
365+
func TestUpdateRegistry_BlockedByPolicyGate(t *testing.T) {
366+
original := registry.ActivePolicyGate()
367+
t.Cleanup(func() { registry.RegisterPolicyGate(original) })
368+
369+
sentinel := errors.New("[ToolHive Policy] Registry is managed by organization policy")
370+
registry.RegisterPolicyGate(&denyRegistryGate{err: sentinel})
371+
372+
provider, cleanup := CreateTestConfigProvider(t, nil)
373+
defer cleanup()
374+
routes := NewRegistryRoutesWithProvider(provider)
375+
376+
body := `{"url":"https://example.com/registry.json"}`
377+
req := httptest.NewRequest(http.MethodPut, "/default", strings.NewReader(body))
378+
req.Header.Set("Content-Type", "application/json")
379+
rctx := chi.NewRouteContext()
380+
rctx.URLParams.Add("name", "default")
381+
req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx))
382+
383+
w := httptest.NewRecorder()
384+
routes.updateRegistry(w, req)
385+
386+
assert.Equal(t, http.StatusForbidden, w.Code, "Blocked PUT should return 403")
387+
assert.Contains(t, w.Body.String(), "organization policy")
388+
}
389+
390+
//nolint:paralleltest // Mutates global registry policy gate singleton
391+
func TestRemoveRegistry_BlockedByPolicyGate(t *testing.T) {
392+
original := registry.ActivePolicyGate()
393+
t.Cleanup(func() { registry.RegisterPolicyGate(original) })
394+
395+
sentinel := errors.New("[ToolHive Policy] Registry is managed by organization policy")
396+
registry.RegisterPolicyGate(&denyRegistryGate{err: sentinel})
397+
398+
routes := &RegistryRoutes{}
399+
400+
req := httptest.NewRequest(http.MethodDelete, "/default", nil)
401+
rctx := chi.NewRouteContext()
402+
rctx.URLParams.Add("name", "default")
403+
req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx))
404+
405+
w := httptest.NewRecorder()
406+
routes.removeRegistry(w, req)
407+
408+
assert.Equal(t, http.StatusForbidden, w.Code, "Blocked DELETE should return 403")
409+
assert.Contains(t, w.Body.String(), "organization policy")
410+
}
411+
412+
//nolint:paralleltest // Mutates global registry policy gate singleton
413+
func TestUpdateRegistry_AllowedByDefaultGate(t *testing.T) {
414+
original := registry.ActivePolicyGate()
415+
t.Cleanup(func() { registry.RegisterPolicyGate(original) })
416+
417+
// Reset to default (allow-all) gate
418+
registry.RegisterPolicyGate(registry.NoopPolicyGate{})
419+
420+
provider, cleanup := CreateTestConfigProvider(t, nil)
421+
defer cleanup()
422+
routes := NewRegistryRoutesWithProvider(provider)
423+
424+
// Empty body resets registry — should return 200 when gate allows
425+
body := `{}`
426+
req := httptest.NewRequest(http.MethodPut, "/default", strings.NewReader(body))
427+
req.Header.Set("Content-Type", "application/json")
428+
rctx := chi.NewRouteContext()
429+
rctx.URLParams.Add("name", "default")
430+
req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx))
431+
432+
w := httptest.NewRecorder()
433+
routes.updateRegistry(w, req)
434+
435+
assert.NotEqual(t, http.StatusForbidden, w.Code,
436+
"Default gate should not return 403")
437+
}

pkg/registry/policy_gate.go

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package registry
5+
6+
import (
7+
"context"
8+
"sync"
9+
)
10+
11+
// UpdateRegistryConfig contains the configuration for a registry update,
12+
// used by both CLI and API callers for policy evaluation. At most one of URL,
13+
// APIURL, or LocalPath is set.
14+
type UpdateRegistryConfig struct {
15+
// URL is the remote registry file URL being set.
16+
URL string
17+
// APIURL is the MCP Registry API endpoint URL being set.
18+
APIURL string
19+
// LocalPath is the local registry file path being set.
20+
LocalPath string
21+
// AllowPrivateIP indicates whether private IP addresses are permitted.
22+
AllowPrivateIP bool
23+
// HasAuth indicates whether authentication is being configured.
24+
HasAuth bool
25+
}
26+
27+
// DeleteRegistryConfig contains the configuration for a registry deletion,
28+
// used by both CLI and API callers for policy evaluation.
29+
type DeleteRegistryConfig struct {
30+
// Name is the registry name being removed (e.g. "default").
31+
Name string
32+
}
33+
34+
// PolicyGate is called before registry mutation operations to allow external
35+
// policy enforcement. Downstream implementations should embed NoopPolicyGate
36+
// to remain forward-compatible when new methods are added.
37+
//
38+
// Error messages returned from the check methods are surfaced directly to the
39+
// end user (HTTP response body or CLI stderr). The policy gate implementer is
40+
// responsible for producing clear, actionable messages.
41+
type PolicyGate interface {
42+
// CheckUpdateRegistry is called before a registry is created or updated.
43+
// Return a non-nil error to block the operation.
44+
CheckUpdateRegistry(ctx context.Context, cfg *UpdateRegistryConfig) error
45+
46+
// CheckDeleteRegistry is called before a registry is deleted or unset.
47+
// Return a non-nil error to block the operation.
48+
CheckDeleteRegistry(ctx context.Context, cfg *DeleteRegistryConfig) error
49+
}
50+
51+
// NoopPolicyGate is a policy gate that allows all registry mutations.
52+
// Downstream implementations should embed this struct to remain
53+
// forward-compatible when new methods are added to the PolicyGate interface.
54+
type NoopPolicyGate struct{}
55+
56+
// CheckUpdateRegistry implements PolicyGate by allowing all updates.
57+
func (NoopPolicyGate) CheckUpdateRegistry(_ context.Context, _ *UpdateRegistryConfig) error {
58+
return nil
59+
}
60+
61+
// CheckDeleteRegistry implements PolicyGate by allowing all deletions.
62+
func (NoopPolicyGate) CheckDeleteRegistry(_ context.Context, _ *DeleteRegistryConfig) error {
63+
return nil
64+
}
65+
66+
// allowAllGate is the default policy gate used when no gate has been registered.
67+
type allowAllGate struct {
68+
NoopPolicyGate
69+
}
70+
71+
var (
72+
regGateMu sync.RWMutex
73+
regGate PolicyGate = allowAllGate{}
74+
)
75+
76+
// RegisterPolicyGate replaces the active registry policy gate with g. It is
77+
// safe to call from multiple goroutines, though it is intended to be called
78+
// once at program startup.
79+
func RegisterPolicyGate(g PolicyGate) {
80+
regGateMu.Lock()
81+
defer regGateMu.Unlock()
82+
regGate = g
83+
}
84+
85+
// ActivePolicyGate returns the currently registered registry policy gate.
86+
func ActivePolicyGate() PolicyGate {
87+
regGateMu.RLock()
88+
defer regGateMu.RUnlock()
89+
return regGate
90+
}

0 commit comments

Comments
 (0)