Skip to content

Commit d89e4be

Browse files
scotwellsclaude
andcommitted
fix(plugin): enforce the catalog allow-list continuously and scope GitHub by owner
An organization's plugin catalog allow-list was only checked when a catalog was first added, so a catalog added before a policy was published kept working forever. The allow-list is now applied every time the catalog registry loads: a catalog whose source is no longer permitted is disabled and excluded from search, browse, install, and resolution, with a clear warning explaining why. Disabled catalogs are still listed (marked disabled) and can still be removed, and the official and organization- managed catalogs are never disabled. With no allow-list configured nothing changes. Allow-list entries can now scope GitHub sources by owner or repository (github.com/<owner>/*, github.com/<owner>, or github.com/<owner>/<repo>) instead of authorizing every GitHub repository at once. A plain raw.githubusercontent.com host entry no longer blanket-authorizes GitHub, so the scoping is meaningful; local-only and non-GitHub host rules are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent dfa636f commit d89e4be

9 files changed

Lines changed: 380 additions & 20 deletions

File tree

internal/cmd/plugin/browse.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,16 @@ func browseCmd() *cobra.Command {
5858
return fmt.Errorf("load catalog registry: %w", err)
5959
}
6060

61-
catalogs := reg.Catalogs
61+
warnDisabledCatalogs(cmd.ErrOrStderr(), reg)
62+
catalogs := reg.Active()
6263
if indexName != "" {
6364
cat := reg.Find(indexName)
6465
if cat == nil {
6566
return customerrors.NewUserError(fmt.Sprintf("catalog %q is not registered", indexName))
6667
}
68+
if cat.Disabled {
69+
return customerrors.NewUserError(fmt.Sprintf("catalog %q is disabled: %s", indexName, cat.DisabledReason))
70+
}
6771
catalogs = []pluginstore.Catalog{*cat}
6872
}
6973

internal/cmd/plugin/catalog_resolve.go

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package plugin
22

33
import (
44
"fmt"
5+
"io"
56
"runtime"
67
"strings"
78

@@ -10,6 +11,18 @@ import (
1011
"go.datum.net/datumctl/internal/pluginstore"
1112
)
1213

14+
// warnDisabledCatalogs prints a one-line warning for each catalog an active
15+
// enterprise allow-list has disabled, so a catalog that silently stops being
16+
// used is explained rather than appearing to vanish. Each disabled catalog is
17+
// named once. It is the single user-facing chokepoint for this notice; the
18+
// commands that resolve/search/install/browse call it after loading the
19+
// registry, then operate on reg.Active().
20+
func warnDisabledCatalogs(w io.Writer, reg *pluginstore.Registry) {
21+
for _, c := range reg.DisabledCatalogs() {
22+
fmt.Fprintf(w, "warning: catalog %q is disabled — %s\n", c.Name, c.DisabledReason)
23+
}
24+
}
25+
1326
// pluginBinaryPrefixes are the recognized datumctl plugin binary prefixes, in
1427
// preference order. "milo-" identifies portable milo-os platform plugins;
1528
// "datumctl-" identifies datumctl-native plugins.
@@ -63,8 +76,9 @@ func loadOrRefreshCatalog(cmd *cobra.Command, pluginsDir string, cat pluginstore
6376
// is skipped with a warning rather than failing the whole resolution.
6477
func resolveBareName(cmd *cobra.Command, pluginsDir string, reg *pluginstore.Registry, name string) []catalogMatch {
6578
var matches []catalogMatch
66-
for i := range reg.Catalogs {
67-
cat := reg.Catalogs[i]
79+
active := reg.Active()
80+
for i := range active {
81+
cat := active[i]
6882
idx, err := loadOrRefreshCatalog(cmd, pluginsDir, cat)
6983
if err != nil {
7084
fmt.Fprintf(cmd.ErrOrStderr(), "warning: skipping catalog %q: %v\n", cat.Name, err)

internal/cmd/plugin/index.go

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,11 @@ func indexListCmd() *cobra.Command {
206206
var wg sync.WaitGroup
207207
for i := range reg.Catalogs {
208208
cat := reg.Catalogs[i]
209+
if cat.Disabled {
210+
// A disabled catalog is excluded from use; don't fetch it. It is
211+
// still listed below, marked disabled, so the state is visible.
212+
continue
213+
}
209214
cached, _ := pluginstore.LoadCatalogIndex(pluginsDir, cat.Name)
210215
if !pluginstore.IsStale(cached) {
211216
indexes[i] = cached
@@ -237,6 +242,17 @@ func indexListCmd() *cobra.Command {
237242
desc = idx.Header.Description
238243
}
239244
}
245+
if cat.Disabled {
246+
// Mark a disabled catalog inline so the user understands why it no
247+
// longer participates in search/install rather than seeing it vanish.
248+
count = "—"
249+
marker := "(disabled: not in allow-list)"
250+
if desc != "" {
251+
desc = marker + " " + desc
252+
} else {
253+
desc = marker
254+
}
255+
}
240256
fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\n", cat.Name, cat.Type, count, cat.Trust(), desc)
241257
}
242258
return w.Flush()
@@ -329,15 +345,21 @@ func indexUpdateCmd() *cobra.Command {
329345
return fmt.Errorf("load catalog registry: %w", err)
330346
}
331347

348+
warnDisabledCatalogs(cmd.ErrOrStderr(), reg)
349+
332350
var targets []pluginstore.Catalog
333351
if len(args) == 1 {
334352
cat := reg.Find(args[0])
335353
if cat == nil {
336354
return customerrors.NewUserError(fmt.Sprintf("catalog %q is not registered", args[0]))
337355
}
356+
if cat.Disabled {
357+
return customerrors.NewUserError(fmt.Sprintf("catalog %q is disabled: %s", args[0], cat.DisabledReason))
358+
}
338359
targets = []pluginstore.Catalog{*cat}
339360
} else {
340-
targets = reg.Catalogs
361+
// Disabled catalogs are excluded from use, so don't refresh them.
362+
targets = reg.Active()
341363
}
342364

343365
var failures int

internal/cmd/plugin/index_test.go

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,82 @@ func TestIndexRemove_managedRejected(t *testing.T) {
317317
}
318318
}
319319

320+
// registerLocalCatalog persists a single user catalog pointing at a local
321+
// manifest, bypassing the `add` allow-list gate, so load-time enforcement can be
322+
// exercised on its own.
323+
func registerLocalCatalog(t *testing.T, pluginsDir, name string) {
324+
t.Helper()
325+
src := writeLocalCatalog(t, testCatalogManifest)
326+
if err := pluginstore.SaveRegistry(pluginsDir, &pluginstore.Registry{Catalogs: []pluginstore.Catalog{
327+
{Name: name, Source: src, Type: pluginstore.CatalogTypeCustom},
328+
}}); err != nil {
329+
t.Fatal(err)
330+
}
331+
}
332+
333+
func TestIndexList_showsDisabledCatalog(t *testing.T) {
334+
pluginsDir := t.TempDir()
335+
seedFreshCache(t, pluginsDir, "datum", testPlugin("dns", "v1.2.3", "Manage DNS zones"))
336+
registerLocalCatalog(t, pluginsDir, "acme")
337+
338+
// An allow-list that does not cover acme disables it at load time.
339+
t.Setenv("DATUMCTL_PLUGIN_ALLOWED_INDEXES", "approved-only")
340+
out, err := runIndex(t, pluginsDir, "", "list")
341+
if err != nil {
342+
t.Fatalf("list must not fail with a disabled catalog: %v", err)
343+
}
344+
if !strings.Contains(out, "acme") {
345+
t.Fatalf("disabled catalog must still be listed: %s", out)
346+
}
347+
for _, line := range strings.Split(out, "\n") {
348+
if strings.HasPrefix(line, "acme") && !strings.Contains(line, "disabled") {
349+
t.Fatalf("acme row should be marked disabled, got: %q", line)
350+
}
351+
}
352+
}
353+
354+
func TestIndexRemove_removesDisabledCatalog(t *testing.T) {
355+
pluginsDir := t.TempDir()
356+
registerLocalCatalog(t, pluginsDir, "acme")
357+
t.Setenv("DATUMCTL_PLUGIN_ALLOWED_INDEXES", "approved-only")
358+
359+
// A disabled catalog can still be cleaned up.
360+
out, err := runIndex(t, pluginsDir, "", "remove", "acme")
361+
if err != nil {
362+
t.Fatalf("removing a disabled catalog should succeed: %v", err)
363+
}
364+
if !strings.Contains(out, "Removed catalog acme") {
365+
t.Fatalf("unexpected output: %s", out)
366+
}
367+
if reg, _ := pluginstore.LoadRegistry(pluginsDir); reg.Find("acme") != nil {
368+
t.Fatal("acme should be gone after remove")
369+
}
370+
}
371+
372+
func TestSearch_excludesDisabledCatalogAndWarns(t *testing.T) {
373+
pluginsDir := t.TempDir()
374+
seedFreshCache(t, pluginsDir, "datum", testPlugin("dns", "v1.2.3", "Manage DNS zones"))
375+
registerLocalCatalog(t, pluginsDir, "acme")
376+
// Pre-seed acme's cache with its "deploy" plugin so a network fetch isn't
377+
// needed to prove it would otherwise appear.
378+
seedFreshCache(t, pluginsDir, "acme", testPlugin("deploy", "v2.1.0", "ACME guided deploy"))
379+
t.Setenv("DATUMCTL_PLUGIN_ALLOWED_INDEXES", "approved-only")
380+
381+
out, err := execPluginCmd(t, pluginsDir, searchCmd())
382+
if err != nil {
383+
t.Fatalf("search must not fail: %v", err)
384+
}
385+
if !strings.Contains(out, "dns") {
386+
t.Fatalf("the permitted datum catalog's plugin should appear: %s", out)
387+
}
388+
if strings.Contains(out, "deploy") {
389+
t.Fatalf("a disabled catalog's plugin must not appear in search: %s", out)
390+
}
391+
if !strings.Contains(out, "disabled") || !strings.Contains(out, "acme") {
392+
t.Fatalf("search should warn that acme is disabled: %s", out)
393+
}
394+
}
395+
320396
func TestIndexValidate_goodAndBad(t *testing.T) {
321397
pluginsDir := t.TempDir()
322398

internal/cmd/plugin/install.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ The plugin binary is written to the managed plugins directory
7474
if err != nil {
7575
return fmt.Errorf("load catalog registry: %w", err)
7676
}
77+
warnDisabledCatalogs(cmd.ErrOrStderr(), reg)
7778
return installArg(cmd, pluginsDir, reg, args[0], currentVersion)
7879
},
7980
}
@@ -182,6 +183,9 @@ func installFromCatalog(cmd *cobra.Command, pluginsDir string, reg *pluginstore.
182183
if cat == nil {
183184
return customerrors.NewUserError(fmt.Sprintf("catalog %q is not registered; run 'datumctl plugin index add %s <source>' first", catalogName, catalogName))
184185
}
186+
if cat.Disabled {
187+
return customerrors.NewUserError(fmt.Sprintf("catalog %q is disabled: %s", catalogName, cat.DisabledReason))
188+
}
185189
idx, err := loadOrRefreshCatalog(cmd, pluginsDir, *cat)
186190
if err != nil {
187191
return indexFetchUserError(err)
@@ -262,6 +266,7 @@ func installAllFromManifest(cmd *cobra.Command, pluginsDir, currentVersion strin
262266
if err != nil {
263267
return fmt.Errorf("load catalog registry: %w", err)
264268
}
269+
warnDisabledCatalogs(cmd.ErrOrStderr(), reg)
265270

266271
// Lazily load+refresh each catalog index, memoized by catalog name.
267272
indexCache := map[string]*pluginstore.CachedIndex{}
@@ -273,6 +278,9 @@ func installAllFromManifest(cmd *cobra.Command, pluginsDir, currentVersion strin
273278
if cat == nil {
274279
return nil, fmt.Errorf("catalog %q is no longer registered", catalogName)
275280
}
281+
if cat.Disabled {
282+
return nil, fmt.Errorf("catalog %q is disabled: %s", catalogName, cat.DisabledReason)
283+
}
276284
idx, loadErr := loadOrRefreshCatalog(cmd, pluginsDir, *cat)
277285
if loadErr != nil {
278286
return nil, loadErr

internal/cmd/plugin/search.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,16 @@ func searchCmd() *cobra.Command {
4747
return fmt.Errorf("load catalog registry: %w", err)
4848
}
4949

50-
catalogs := reg.Catalogs
50+
warnDisabledCatalogs(cmd.ErrOrStderr(), reg)
51+
catalogs := reg.Active()
5152
if indexName != "" {
5253
cat := reg.Find(indexName)
5354
if cat == nil {
5455
return customerrors.NewUserError(fmt.Sprintf("catalog %q is not registered", indexName))
5556
}
57+
if cat.Disabled {
58+
return customerrors.NewUserError(fmt.Sprintf("catalog %q is disabled: %s", indexName, cat.DisabledReason))
59+
}
5660
catalogs = []pluginstore.Catalog{*cat}
5761
}
5862

internal/pluginstore/catalog.go

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,15 @@ type Catalog struct {
8484
// Managed catalogs are not persisted to indexes.json and cannot be removed
8585
// by the user. Not serialized.
8686
Managed bool `json:"-"`
87+
88+
// Disabled marks a catalog whose source is no longer permitted by an active
89+
// enterprise allow-list. A disabled catalog is still shown by `index list`
90+
// and can be removed, but is excluded from resolve, search, install, and
91+
// browse. Set at load time by ApplyAllowList. Not serialized.
92+
Disabled bool `json:"-"`
93+
// DisabledReason is the human-readable reason a catalog is disabled. Not
94+
// serialized.
95+
DisabledReason string `json:"-"`
8796
}
8897

8998
// IsOfficial reports whether this is the reserved official catalog.
@@ -130,6 +139,31 @@ func (r *Registry) Find(name string) *Catalog {
130139
return nil
131140
}
132141

142+
// Active returns the catalogs available for resolve, search, install, and
143+
// browse — every registered catalog except those an active allow-list has
144+
// disabled.
145+
func (r *Registry) Active() []Catalog {
146+
out := make([]Catalog, 0, len(r.Catalogs))
147+
for _, c := range r.Catalogs {
148+
if !c.Disabled {
149+
out = append(out, c)
150+
}
151+
}
152+
return out
153+
}
154+
155+
// DisabledCatalogs returns the catalogs an active allow-list has disabled, each
156+
// listed once. Used to explain to the user why a catalog stopped being used.
157+
func (r *Registry) DisabledCatalogs() []Catalog {
158+
var out []Catalog
159+
for _, c := range r.Catalogs {
160+
if c.Disabled {
161+
out = append(out, c)
162+
}
163+
}
164+
return out
165+
}
166+
133167
// Custom returns the user-registered catalogs (excludes default and managed).
134168
// These are the entries persisted to indexes.json.
135169
func (r *Registry) Custom() []Catalog {
@@ -240,9 +274,42 @@ func LoadRegistry(pluginsDir string) (*Registry, error) {
240274
add(c)
241275
}
242276

277+
// Enforce the allow-list as an ongoing guardrail, not just an onboarding
278+
// gate: a user-registered catalog whose source is no longer permitted is
279+
// marked disabled so it is excluded from use.
280+
reg.ApplyAllowList(managed)
281+
243282
return reg, nil
244283
}
245284

285+
// allowListDisabledReason explains why a catalog was disabled by ApplyAllowList.
286+
const allowListDisabledReason = "its source is no longer permitted by your organization's plugin catalog allow-list"
287+
288+
// ApplyAllowList marks every user-registered catalog whose source is not
289+
// permitted by an active allow-list as disabled and returns the disabled
290+
// catalogs. The official catalog and managed pre-seeds are admin-controlled and
291+
// are never disabled. When no allow-list is configured nothing is disabled, so
292+
// the common default case is unchanged. LoadRegistry calls this so enforcement
293+
// is centralized at load time rather than only at `index add`.
294+
func (r *Registry) ApplyAllowList(m *ManagedConfig) []Catalog {
295+
if m == nil || !m.Enforced() {
296+
return nil
297+
}
298+
var disabled []Catalog
299+
for i := range r.Catalogs {
300+
c := &r.Catalogs[i]
301+
if c.Managed || CanonicalCatalogName(c.Name) == OfficialCatalogName {
302+
continue
303+
}
304+
if !m.IsAllowed(c.Name, c.Source) {
305+
c.Disabled = true
306+
c.DisabledReason = allowListDisabledReason
307+
disabled = append(disabled, *c)
308+
}
309+
}
310+
return disabled
311+
}
312+
246313
// SaveRegistry persists only the user-registered catalogs (default and managed
247314
// entries are never written to disk).
248315
func SaveRegistry(pluginsDir string, reg *Registry) error {
@@ -382,3 +449,31 @@ func SourceHost(source string) string {
382449
}
383450
return u.Hostname()
384451
}
452+
453+
// SourceGitHubOwnerRepo returns the GitHub "owner/repo" a catalog source
454+
// targets, for any of the forms that resolve to GitHub: the "owner/repo" (and
455+
// "github.com/owner/repo") shorthand, and a full raw.githubusercontent.com
456+
// manifest URL. ok is false for non-GitHub sources. Used to scope GitHub
457+
// allow-list entries by owner/repo.
458+
func SourceGitHubOwnerRepo(source string) (ownerRepo string, ok bool) {
459+
resolved, err := ResolveCatalogSource(source)
460+
if err != nil || resolved.IsLocal {
461+
return "", false
462+
}
463+
if resolved.GitHubOwnerRepo != "" {
464+
return resolved.GitHubOwnerRepo, true
465+
}
466+
u, err := url.Parse(resolved.FetchURL)
467+
if err != nil {
468+
return "", false
469+
}
470+
host := strings.ToLower(u.Hostname())
471+
if host != "raw.githubusercontent.com" && host != "github.com" {
472+
return "", false
473+
}
474+
parts := strings.SplitN(strings.TrimPrefix(u.Path, "/"), "/", 3)
475+
if len(parts) >= 2 && parts[0] != "" && parts[1] != "" {
476+
return parts[0] + "/" + parts[1], true
477+
}
478+
return "", false
479+
}

0 commit comments

Comments
 (0)