From ce9aff21f1add8f96966272df3fd0d4d99cc421f Mon Sep 17 00:00:00 2001 From: Ed Snible Date: Wed, 2 Sep 2026 11:33:03 -0400 Subject: [PATCH] feat: Declare plugin pipeline directions and complete ConfigSchema coverage Make plugin placement and config metadata machine-readable, so config generators no longer have to infer either from source or from a hand-maintained table. Directions (new PluginCapabilities field): Every plugin has an intended chain -- jwt-validation is inbound, token-exchange outbound, opa both -- but that lived only in the Direction column of docs/plugin-catalog.md. All 14 in-tree plugins now declare it as Capabilities().Directions, published on /v1/plugins and /v1/pipeline as `directions`, and asserted by a test so the docs table has a source of truth. Advisory, never fatal: no plugin enforces direction at runtime (opa, the one that cares, merely branches on pctx.Direction), so a misplaced plugin is a probable misconfiguration rather than a guaranteed one, and failing the boot would break configs that work today. A mismatch logs a startup WARN (plugins.WarnPluginDirections, called beside the existing WarnEmptyPipelines) and shows an advisory in abctl. Nil Directions means unconstrained, so out-of-tree plugins are unaffected. The wire field is plural and string-typed on purpose. `direction` (singular) already means "the chain this configured instance sits in" and stays untouched; `directions` is the type-level set of chains a plugin supports. Strings rather than []Direction because Direction.UnmarshalJSON decodes any unknown value to Inbound without erroring, which on a slice would turn a future third value into a false "inbound" claim. ConfigSchema: mcp-parser, opa, session-budget and litellm-budget-track were Configurable but not SchemaProvider, so /v1/plugins reported no field metadata and abctl rendered them as bare names. All four now implement it; mcp-parser and opa also gained the field annotations (defaults and required flags taken from their applyDefaults/Configure, not guessed). a2a-parser and inference-parser are deliberately excluded -- they have no config at all, and pipeline/schema.go names them as legitimate omissions. Also adds a "number" schema type for floats. litellm-budget-track is the first plugin to expose float config; without it max_budget and the five per-token rates would publish as "unknown" and render as quoted empty strings in templates. abctl: Templates carry a `# chain:` line per plugin, and the pre-apply validator flags a plugin pasted into a chain it doesn't declare. ValidationError gains a Severity so advisories render under their own banner -- folding them into the existing one would make its "framework reload will reject" claim false. Assisted-By: Claude (Anthropic AI) Signed-off-by: Ed Snible --- authbridge/CLAUDE.md | 4 +- .../authlib/pipeline/directions_test.go | 96 +++++++++ authbridge/authlib/pipeline/plugin.go | 69 +++++- authbridge/authlib/pipeline/schema.go | 7 +- authbridge/authlib/pipeline/schema_test.go | 25 +++ .../authlib/plugins/a2aparser/plugin.go | 1 + .../authlib/plugins/configschema_test.go | 169 +++++++++++++++ .../authlib/plugins/contextguru/plugin.go | 3 +- authbridge/authlib/plugins/cpex/plugin.go | 1 + authbridge/authlib/plugins/directions_test.go | 203 ++++++++++++++++++ authbridge/authlib/plugins/ibac/plugin.go | 1 + .../authlib/plugins/inferenceparser/plugin.go | 1 + .../authlib/plugins/jwtvalidation/plugin.go | 1 + .../plugins/litellm_budgettrack/plugin.go | 7 + .../authlib/plugins/mcpparser/plugin.go | 9 +- authbridge/authlib/plugins/opa/plugin.go | 35 ++- authbridge/authlib/plugins/registry.go | 1 + .../authlib/plugins/sessionbudget/plugin.go | 7 + authbridge/authlib/plugins/sparc/plugin.go | 1 + .../authlib/plugins/staticinject/plugin.go | 1 + .../authlib/plugins/tokenbroker/plugin.go | 1 + .../authlib/plugins/tokenexchange/plugin.go | 1 + authbridge/authlib/plugins/warn.go | 89 ++++++++ .../authlib/sessionapi/catalog_adapter.go | 36 +++- authbridge/authlib/sessionapi/server.go | 14 +- authbridge/cmd/abctl/README.md | 22 +- authbridge/cmd/abctl/apiclient/client.go | 7 +- authbridge/cmd/abctl/apiclient/client_test.go | 52 +++++ authbridge/cmd/abctl/edit/templates.go | 15 ++ authbridge/cmd/abctl/edit/templates_test.go | 51 ++++- authbridge/cmd/abctl/edit/validate.go | 38 ++++ authbridge/cmd/abctl/edit/validate_test.go | 111 ++++++++++ authbridge/cmd/abctl/tui/catalog_pane.go | 10 +- authbridge/cmd/abctl/tui/edit_overlay.go | 51 ++++- authbridge/cmd/authbridge-cpex/main.go | 1 + authbridge/cmd/authbridge-envoy/main.go | 1 + .../cmd/authbridge-proxy/catalog_test.go | 95 ++++++++ authbridge/cmd/authbridge-proxy/main.go | 1 + authbridge/docs/plugin-catalog.md | 8 +- 39 files changed, 1213 insertions(+), 33 deletions(-) create mode 100644 authbridge/authlib/pipeline/directions_test.go create mode 100644 authbridge/authlib/plugins/configschema_test.go create mode 100644 authbridge/authlib/plugins/directions_test.go create mode 100644 authbridge/authlib/plugins/warn.go create mode 100644 authbridge/cmd/authbridge-proxy/catalog_test.go diff --git a/authbridge/CLAUDE.md b/authbridge/CLAUDE.md index d79a4bb8f..61be0a462 100644 --- a/authbridge/CLAUDE.md +++ b/authbridge/CLAUDE.md @@ -425,8 +425,8 @@ When `session.enabled` is true (default) and `listener.session_api_addr` is non- | `GET /v1/sessions` | `application/json` | List active sessions: `{sessions: [{id, createdAt, updatedAt, eventCount, active}]}`. | | `GET /v1/sessions/{id}` | `application/json` | Full snapshot of one session's events. 404 if unknown/expired. | | `GET /v1/events` | `text/event-stream` | SSE stream of new events. Optional `?session=` filters to one session. Heartbeat every 30s. | -| `GET /v1/pipeline` | `application/json` | Active pipeline composition: `{inbound: [...], outbound: [...]}`. Each plugin entry carries `name`, `direction`, `position`, `readsBody`, plus the static metadata (`requires`, `requiresAny`, `description`) and runtime `config` when present. abctl renders this as the Pipeline pane. | -| `GET /v1/plugins` | `application/json` | Catalog of every registered plugin (whether or not in the active pipeline): `{plugins: [{name, requires, requiresAny, description, ...}]}`. abctl renders this as the Catalog pane (`P` key). 404s when the binary's session API was constructed without `WithCatalog`. | +| `GET /v1/pipeline` | `application/json` | Active pipeline composition: `{inbound: [...], outbound: [...]}`. Each plugin entry carries `name`, `direction` (the chain this instance sits in), `position`, `readsBody`, plus the static metadata (`requires`, `requiresAny`, `directions`, `description`) and runtime `config` when present. abctl renders this as the Pipeline pane. | +| `GET /v1/plugins` | `application/json` | Catalog of every registered plugin (whether or not in the active pipeline): `{plugins: [{name, directions, requires, requiresAny, description, fields, ...}]}`. `directions` is the type-level list of chains the plugin supports (`["inbound"]`, `["outbound"]`, or both; absent = unconstrained) — distinct from `/v1/pipeline`'s positional `direction`, and what config generators read to place a plugin. `fields` carries per-field config schema for plugins implementing `pipeline.SchemaProvider`. abctl renders this as the Catalog pane (`P` key). 404s when the binary's session API was constructed without `WithCatalog`. | | `GET /healthz` | text | Liveness probe. | ### Quick examples diff --git a/authbridge/authlib/pipeline/directions_test.go b/authbridge/authlib/pipeline/directions_test.go new file mode 100644 index 000000000..88e07f8e2 --- /dev/null +++ b/authbridge/authlib/pipeline/directions_test.go @@ -0,0 +1,96 @@ +package pipeline + +import ( + "reflect" + "testing" +) + +// A plugin that declares nothing is unconstrained: Supports must answer +// "no objection" for every direction, which is what keeps the field +// advisory and backward-compatible with out-of-tree plugins. +func TestSupportsUnconstrained(t *testing.T) { + var caps PluginCapabilities + for _, d := range []Direction{Inbound, Outbound} { + if !caps.Supports(d) { + t.Errorf("nil Directions should support %s", d) + } + } + // An explicitly-empty slice behaves the same as nil. + caps.Directions = []Direction{} + for _, d := range []Direction{Inbound, Outbound} { + if !caps.Supports(d) { + t.Errorf("empty Directions should support %s", d) + } + } +} + +func TestSupports(t *testing.T) { + cases := []struct { + name string + declared []Direction + wantIn bool + wantOutbnd bool + }{ + {"inbound only", []Direction{Inbound}, true, false}, + {"outbound only", []Direction{Outbound}, false, true}, + {"both", []Direction{Inbound, Outbound}, true, true}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + caps := PluginCapabilities{Directions: c.declared} + if got := caps.Supports(Inbound); got != c.wantIn { + t.Errorf("Supports(Inbound) = %v, want %v", got, c.wantIn) + } + if got := caps.Supports(Outbound); got != c.wantOutbnd { + t.Errorf("Supports(Outbound) = %v, want %v", got, c.wantOutbnd) + } + }) + } +} + +// Normalize canonicalizes Directions so two literals describing the same +// plugin can't produce two different cached/wire representations. +func TestNormalizeDirections(t *testing.T) { + cases := []struct { + name string + in []Direction + want []Direction + }{ + {"nil stays nil", nil, nil}, + {"empty becomes nil", []Direction{}, nil}, + {"sorts", []Direction{Outbound, Inbound}, []Direction{Inbound, Outbound}}, + {"dedups", []Direction{Inbound, Inbound}, []Direction{Inbound}}, + {"dedups and sorts", []Direction{Outbound, Inbound, Outbound}, []Direction{Inbound, Outbound}}, + {"already canonical", []Direction{Inbound, Outbound}, []Direction{Inbound, Outbound}}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := PluginCapabilities{Directions: c.in}.Normalize().Directions + if !reflect.DeepEqual(got, c.want) { + t.Errorf("Normalize().Directions = %v, want %v", got, c.want) + } + }) + } +} + +// Normalize must not reorder the caller's slice: a plugin returning a +// package-level slice from Capabilities() would otherwise have it +// permuted underneath it by whoever normalized first. +func TestNormalizeDoesNotMutateInput(t *testing.T) { + orig := []Direction{Outbound, Inbound} + caps := PluginCapabilities{Directions: orig} + _ = caps.Normalize() + if orig[0] != Outbound || orig[1] != Inbound { + t.Fatalf("Normalize mutated the input slice: %v", orig) + } +} + +// Normalize is idempotent — the catalog normalizes on read, so applying +// it twice must not change the answer. +func TestNormalizeDirectionsIdempotent(t *testing.T) { + once := PluginCapabilities{Directions: []Direction{Outbound, Inbound, Inbound}}.Normalize() + twice := once.Normalize() + if !reflect.DeepEqual(once.Directions, twice.Directions) { + t.Errorf("not idempotent: %v then %v", once.Directions, twice.Directions) + } +} diff --git a/authbridge/authlib/pipeline/plugin.go b/authbridge/authlib/pipeline/plugin.go index a00a1e5ac..063be3a53 100644 --- a/authbridge/authlib/pipeline/plugin.go +++ b/authbridge/authlib/pipeline/plugin.go @@ -1,6 +1,9 @@ package pipeline -import "context" +import ( + "context" + "slices" +) // Plugin is the interface that all pipeline extensions implement. type Plugin interface { @@ -60,6 +63,25 @@ type PluginCapabilities struct { // of running the guardrail as silent dead code. RequiresAny []string + // Directions declares which pipeline chains this plugin is designed + // to run in. Advisory metadata only: nothing rejects a plugin that + // is configured into another chain, because no plugin enforces + // direction at runtime (opa, the one plugin that cares, merely + // branches on pctx.Direction). A mismatch produces a startup WARN + // from plugins.WarnPluginDirections and an advisory in abctl's + // pre-apply validator. + // + // Nil or empty means unconstrained — the plugin makes no claim and + // no warning is ever emitted for it. That is the zero value, so + // out-of-tree plugins and test stubs need no change. + // + // This is the machine-readable form of the Direction column in + // authbridge/docs/plugin-catalog.md, and it is what lets config + // generators place a plugin in the right chain without a + // hand-maintained table. Use Supports to test membership rather + // than scanning the slice directly. + Directions []Direction + // Description is operator-facing prose, one line, ≤80 chars, // describing what this plugin does. Surfaces in `abctl`'s // plugin-detail and catalog panes, and in /v1/plugins. @@ -71,17 +93,60 @@ type PluginCapabilities struct { Description string } -// Normalize applies WritesBody-implies-ReadsBody promotion. +// Normalize applies WritesBody-implies-ReadsBody promotion and puts +// Directions into a canonical form (de-duplicated, ascending). // Called by Pipeline.New for every plugin's declared capabilities so the // rest of the framework reads a normalized form. Plugins never need to // call this themselves. +// +// Canonicalizing Directions matters because the catalog is cached and +// compared: a hand-written literal of {Outbound, Inbound} and one of +// {Inbound, Outbound} describe the same plugin and must not produce two +// different wire representations. Normalize copies the slice rather than +// sorting in place, so a plugin returning a shared backing array from +// Capabilities() can't have it reordered underneath it. func (c PluginCapabilities) Normalize() PluginCapabilities { if c.WritesBody { c.ReadsBody = true } + c.Directions = canonicalDirections(c.Directions) return c } +// canonicalDirections returns a de-duplicated, ascending copy of in. +// Returns nil for empty input so the "unconstrained" case stays a nil +// slice all the way to the wire (where it elides via omitempty). +func canonicalDirections(in []Direction) []Direction { + if len(in) == 0 { + return nil + } + out := make([]Direction, 0, len(in)) + for _, d := range in { + if !slices.Contains(out, d) { + out = append(out, d) + } + } + slices.Sort(out) + return out +} + +// Supports reports whether the plugin declares itself usable in the +// given direction. A plugin with no declared Directions is +// unconstrained and supports every direction, so this returns true — +// callers get "no objection" rather than "no support" for the zero +// value, which is what keeps the field advisory and backward +// compatible. +// +// Every consumer (the startup warning, abctl's validator, the template +// renderer) goes through this method so the membership rule lives in +// one place. +func (c PluginCapabilities) Supports(d Direction) bool { + if len(c.Directions) == 0 { + return true + } + return slices.Contains(c.Directions, d) +} + // Initializer is an optional interface a plugin may implement when it // needs to run work once before the pipeline starts serving traffic. // Typical uses: load a model, warm a cache, open a database connection, diff --git a/authbridge/authlib/pipeline/schema.go b/authbridge/authlib/pipeline/schema.go index 3258d82d3..7ceea954f 100644 --- a/authbridge/authlib/pipeline/schema.go +++ b/authbridge/authlib/pipeline/schema.go @@ -54,7 +54,10 @@ type FieldSchema struct { // Type is a coarse-grained category sufficient to render templates // and pick value placeholders. One of: - // "string", "int", "bool", "[]string", "object", "unknown". + // "string", "int", "number", "bool", "[]string", "object", "unknown". + // "number" is a float (per-token costs, budgets); "int" stays + // reserved for integral kinds so a template can emit "0" vs "0.0" + // appropriately. // "object" indicates a nested struct whose fields populate Fields. // "unknown" covers shapes the helper hasn't been taught (maps, // slice-of-struct, etc.); the field still renders but without a @@ -174,6 +177,8 @@ func kindOf(t reflect.Type) string { case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: return "int" + case reflect.Float32, reflect.Float64: + return "number" case reflect.Slice: // Only []string gets a typed tag; other slices are "unknown" // (slice-of-struct, slice-of-map, etc. are rare in plugin diff --git a/authbridge/authlib/pipeline/schema_test.go b/authbridge/authlib/pipeline/schema_test.go index c58576672..a35925865 100644 --- a/authbridge/authlib/pipeline/schema_test.go +++ b/authbridge/authlib/pipeline/schema_test.go @@ -167,3 +167,28 @@ func TestSchemaOf_SelfReferentialIsBounded(t *testing.T) { cur = cur.Fields[1] } } + +// floats is a separate fixture rather than an extension of `primitives` +// so TestSchemaOf_Primitives' exact-match assertion stays untouched. +type floats struct { + Budget float64 `json:"budget" required:"true" description:"Daily budget in USD."` + Rate32 float32 `json:"rate32" description:"Narrower float still maps to number."` + Whole int `json:"whole" description:"Integral kinds must NOT become number."` + AlsoWhole int64 `json:"also_whole"` +} + +// Float fields report "number", distinct from "int", so a template can +// tell a per-token cost from a port number. litellm-budget-track is the +// first plugin to expose float config, which is what motivated the type. +func TestSchemaOf_Floats(t *testing.T) { + got := SchemaOf(floats{}) + want := []FieldSchema{ + {Name: "budget", Type: "number", Required: true, Description: "Daily budget in USD."}, + {Name: "rate32", Type: "number", Description: "Narrower float still maps to number."}, + {Name: "whole", Type: "int", Description: "Integral kinds must NOT become number."}, + {Name: "also_whole", Type: "int"}, + } + if !reflect.DeepEqual(got, want) { + t.Errorf("SchemaOf floats:\n got: %+v\nwant: %+v", got, want) + } +} diff --git a/authbridge/authlib/plugins/a2aparser/plugin.go b/authbridge/authlib/plugins/a2aparser/plugin.go index d9fc8f852..23aca78e9 100644 --- a/authbridge/authlib/plugins/a2aparser/plugin.go +++ b/authbridge/authlib/plugins/a2aparser/plugin.go @@ -27,6 +27,7 @@ func (p *A2AParser) Name() string { return "a2a-parser" } func (p *A2AParser) Capabilities() pipeline.PluginCapabilities { return pipeline.PluginCapabilities{ + Directions: []pipeline.Direction{pipeline.Inbound}, ReadsBody: true, Description: "Parses A2A messages into pctx.Extensions.A2A for downstream plugins.", } diff --git a/authbridge/authlib/plugins/configschema_test.go b/authbridge/authlib/plugins/configschema_test.go new file mode 100644 index 000000000..d46a9c924 --- /dev/null +++ b/authbridge/authlib/plugins/configschema_test.go @@ -0,0 +1,169 @@ +package plugins + +import ( + "encoding/json" + "reflect" + "strings" + "testing" + + "github.com/rossoctl/cortex/authbridge/authlib/pipeline" +) + +// Every config-bearing plugin should publish field metadata, otherwise +// /v1/plugins reports it as a bare name and abctl's templates render it +// with no documented options. +// +// "Config-bearing" is detected structurally: a plugin implementing +// pipeline.Configurable takes a config block, so it should also +// implement SchemaProvider to describe it. Parsers with no config at all +// (a2a-parser, inference-parser) implement neither and are correctly +// exempt without needing an allowlist. +func TestConfigurablePluginsPublishSchema(t *testing.T) { + cat := Catalog() + if len(cat) == 0 { + t.Skip("no plugins linked into this test binary") + } + for _, e := range cat { + factory, ok := factoryFor(e.Name) + if !ok { + t.Errorf("catalog lists %q but no factory resolves it", e.Name) + continue + } + inst := factory() + if _, configurable := inst.(pipeline.Configurable); !configurable { + // No config block; nothing to describe. + if len(e.Fields) > 0 { + t.Errorf("%s publishes %d schema fields but is not Configurable", + e.Name, len(e.Fields)) + } + continue + } + if _, hasSchema := inst.(pipeline.SchemaProvider); !hasSchema { + t.Errorf("%s is Configurable but does not implement ConfigSchema(); "+ + "add `func (p *T) ConfigSchema() []pipeline.FieldSchema { "+ + "return pipeline.SchemaOf(tConfig{}) }`", e.Name) + continue + } + if len(e.Fields) == 0 { + t.Errorf("%s implements SchemaProvider but Catalog() reports no fields", e.Name) + } + } +} + +// A schema field name that doesn't match a real json tag would send +// operators to a key the plugin's decoder rejects (these plugins decode +// with DisallowUnknownFields). Feed every advertised key back through +// Configure to prove the names are real. +// +// Uses factoryFor rather than Build: Build wraps plugins in +// configuredPlugin, which embeds only the narrow pipeline.Plugin +// interface, so neither Configurable nor SchemaProvider survives a type +// assertion through a built instance. Catalog() reads schemas off the raw +// factory instance, which is the path that actually ships. +func TestConfigSchemaFieldNamesDecode(t *testing.T) { + for _, e := range Catalog() { + factory, ok := factoryFor(e.Name) + if !ok || len(e.Fields) == 0 { + continue + } + cfg, ok := factory().(pipeline.Configurable) + if !ok { + continue + } + t.Run(e.Name, func(t *testing.T) { + // Every advertised key, with a type-appropriate zero value. + // A bogus name trips DisallowUnknownFields. + obj := make(map[string]any, len(e.Fields)) + for _, f := range e.Fields { + obj[f.Name] = zeroFor(f) + } + raw, err := json.Marshal(obj) + if err != nil { + t.Fatalf("marshal probe config: %v", err) + } + // Configure may legitimately reject on VALUE grounds (a + // required field left empty, a cross-field rule). What must + // not happen is an unknown-field rejection — that means the + // schema advertises a key the struct doesn't have. + if err := cfg.Configure(raw); err != nil { + if strings.Contains(err.Error(), "unknown field") { + t.Errorf("%s: schema advertises a field the decoder rejects: %v", e.Name, err) + } + } + }) + } +} + +// zeroFor returns a JSON-encodable zero value matching a field's declared +// schema type, so the probe config decodes without type errors. +func zeroFor(f pipeline.FieldSchema) any { + switch f.Type { + case "string": + if len(f.Enum) > 0 { + return f.Enum[0] + } + return "" + case "int": + return 0 + case "number": + return 0.0 + case "bool": + return false + case "[]string": + return []string{} + case "object": + inner := make(map[string]any, len(f.Fields)) + for _, sub := range f.Fields { + inner[sub.Name] = zeroFor(sub) + } + return inner + default: + return nil + } +} + +// The plugins this change wired up report the field count and required +// set their config structs actually have, guarding against a SchemaOf +// pointed at the wrong type. Only asserts plugins present in this test +// binary; TestConfigurablePluginsPublishSchema covers the rest +// structurally. +func TestConfigSchemaShapes(t *testing.T) { + want := map[string]struct { + nFields int + required []string + }{ + "mcp-parser": {1, nil}, + "opa": {6, []string{"bundle_url"}}, + "session-budget": {18, []string{"redis_url"}}, + "litellm-budget-track": {6, []string{"spend_file", "max_budget"}}, + } + seen := 0 + for _, e := range Catalog() { + w, tracked := want[e.Name] + if !tracked { + continue + } + seen++ + t.Run(e.Name, func(t *testing.T) { + if len(e.Fields) != w.nFields { + got := make([]string, len(e.Fields)) + for i, f := range e.Fields { + got[i] = f.Name + } + t.Errorf("got %d fields %v, want %d", len(e.Fields), got, w.nFields) + } + var required []string + for _, f := range e.Fields { + if f.Required { + required = append(required, f.Name) + } + } + if !reflect.DeepEqual(required, w.required) { + t.Errorf("required = %v, want %v", required, w.required) + } + }) + } + if seen == 0 { + t.Skip("none of the tracked plugins are linked into this test binary") + } +} diff --git a/authbridge/authlib/plugins/contextguru/plugin.go b/authbridge/authlib/plugins/contextguru/plugin.go index c016cad08..4fcb0e5a1 100644 --- a/authbridge/authlib/plugins/contextguru/plugin.go +++ b/authbridge/authlib/plugins/contextguru/plugin.go @@ -30,13 +30,13 @@ import ( "github.com/rossoctl/cortex/authbridge/authlib/pipeline" "github.com/rossoctl/cortex/authbridge/authlib/plugins" + bschemas "github.com/maximhq/bifrost/core/schemas" "github.com/rossoctl/context-guru/apply" cgcomponents "github.com/rossoctl/context-guru/components" _ "github.com/rossoctl/context-guru/components/offload" // register offload components _ "github.com/rossoctl/context-guru/components/reformat" // register reformat components cgconfig "github.com/rossoctl/context-guru/config" cgstore "github.com/rossoctl/context-guru/store" - bschemas "github.com/maximhq/bifrost/core/schemas" ) // sentinelHeader is set on the plugin's own outbound LLM calls (via llmclient) so @@ -156,6 +156,7 @@ func (p *ContextGuru) Name() string { return "context-guru" } func (p *ContextGuru) Capabilities() pipeline.PluginCapabilities { return pipeline.PluginCapabilities{ + Directions: []pipeline.Direction{pipeline.Outbound}, ReadsBody: true, WritesBody: true, // single outbound body-writer slot (mutually exclusive with SPARC) RequiresAny: []string{"inference-parser"}, diff --git a/authbridge/authlib/plugins/cpex/plugin.go b/authbridge/authlib/plugins/cpex/plugin.go index bf189ea27..a52dccf3b 100644 --- a/authbridge/authlib/plugins/cpex/plugin.go +++ b/authbridge/authlib/plugins/cpex/plugin.go @@ -118,6 +118,7 @@ func (p *CPEX) Name() string { return "cpex" } // surfaces in the catalog. func (p *CPEX) Capabilities() pipeline.PluginCapabilities { return pipeline.PluginCapabilities{ + Directions: []pipeline.Direction{pipeline.Outbound}, ReadsBody: true, WritesBody: true, RequiresAny: []string{"mcp-parser", "inference-parser", "a2a-parser"}, diff --git a/authbridge/authlib/plugins/directions_test.go b/authbridge/authlib/plugins/directions_test.go new file mode 100644 index 000000000..f38cbc578 --- /dev/null +++ b/authbridge/authlib/plugins/directions_test.go @@ -0,0 +1,203 @@ +package plugins + +import ( + "bytes" + "log/slog" + "strings" + "testing" + + "github.com/rossoctl/cortex/authbridge/authlib/config" + "github.com/rossoctl/cortex/authbridge/authlib/pipeline" +) + +// unconstrainedByDesign lists registered plugins that legitimately +// declare no Directions. Test stubs only — a real plugin that forgets +// the field should fail TestRegisteredPluginsDeclareDirections rather +// than be quietly excused, so keep this list short and justified. +var unconstrainedByDesign = map[string]string{ + "jwt-validation-stub": "plugintesting stub; asserts capability plumbing, not direction", + "token-exchange-stub": "plugintesting stub; asserts capability plumbing, not direction", +} + +// Every real registered plugin declares which chain(s) it belongs in. +// Without this, a newly-added plugin silently reports "unconstrained" +// and config generators lose the placement metadata this field exists +// to provide. +func TestRegisteredPluginsDeclareDirections(t *testing.T) { + cat := Catalog() + if len(cat) == 0 { + t.Skip("no plugins linked into this test binary") + } + for _, e := range cat { + if why, ok := unconstrainedByDesign[e.Name]; ok { + if len(e.Capabilities.Directions) > 0 { + t.Errorf("%s is on the unconstrained allowlist (%s) but declares %v; "+ + "remove it from the allowlist", e.Name, why, e.Capabilities.Directions) + } + continue + } + if len(e.Capabilities.Directions) == 0 { + t.Errorf("plugin %q declares no Directions; add them to its Capabilities() "+ + "(see docs/plugin-catalog.md for the intended chain), or add it to "+ + "unconstrainedByDesign with a reason", e.Name) + } + } +} + +// Catalog() hands out clones. Mutating a returned entry's Directions must +// not taint the memoized snapshot every later caller (and every +// /v1/plugins response) reads. cloneCatalog enumerates capability fields +// by hand, so a new slice field is easy to forget there — this is the +// regression guard for exactly that. +func TestCatalogCloneIsolatesDirections(t *testing.T) { + first := Catalog() + idx := -1 + for i, e := range first { + if len(e.Capabilities.Directions) > 0 { + idx = i + break + } + } + if idx < 0 { + // NOT a skip. Every real plugin declares Directions (see + // TestRegisteredPluginsDeclareDirections), so "nobody has any" + // means the field was dropped somewhere between Capabilities() + // and here — cloneCatalog being the likeliest culprit, since it + // copies capability fields by hand. Skipping would let exactly + // the bug this test exists to catch report a green run. + if len(first) == 0 { + t.Skip("no plugins linked into this test binary") + } + t.Fatal("no catalog entry carries Directions; the field is being dropped " + + "between Capabilities() and Catalog() (check cloneCatalog)") + } + name := first[idx].Name + orig := append([]pipeline.Direction(nil), first[idx].Capabilities.Directions...) + + // Scribble on the caller's copy. + first[idx].Capabilities.Directions[0] = pipeline.Direction(99) + + second := Catalog() + for _, e := range second { + if e.Name != name { + continue + } + if len(e.Capabilities.Directions) != len(orig) { + t.Fatalf("%s: length changed after mutation: got %v, want %v", + name, e.Capabilities.Directions, orig) + } + for i := range orig { + if e.Capabilities.Directions[i] != orig[i] { + t.Errorf("%s: cached catalog was tainted by caller mutation: got %v, want %v", + name, e.Capabilities.Directions, orig) + } + } + return + } + t.Fatalf("plugin %q vanished from the catalog", name) +} + +// warnBuffer returns a logger writing to a buffer plus the buffer. +func warnBuffer() (*slog.Logger, *bytes.Buffer) { + var buf bytes.Buffer + return slog.New(slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelWarn})), &buf +} + +func cfgWith(inbound, outbound []string) *config.Config { + toEntries := func(names []string) []config.PluginEntry { + out := make([]config.PluginEntry, len(names)) + for i, n := range names { + out[i] = config.PluginEntry{Name: n} + } + return out + } + c := &config.Config{} + c.Pipeline.Inbound.Plugins = toEntries(inbound) + c.Pipeline.Outbound.Plugins = toEntries(outbound) + return c +} + +// A plugin placed in a chain it doesn't declare gets a WARN naming both +// the plugin and the mismatch — but the process is expected to continue. +func TestWarnPluginDirectionsFlagsMisplacement(t *testing.T) { + if _, ok := factoryFor("jwt-validation"); !ok { + t.Skip("jwt-validation not linked into this test binary") + } + logger, buf := warnBuffer() + // jwt-validation declares inbound; putting it outbound is the mistake. + WarnPluginDirections(cfgWith(nil, []string{"jwt-validation"}), logger) + + out := buf.String() + if !strings.Contains(out, "jwt-validation") { + t.Errorf("warning should name the plugin:\n%s", out) + } + if !strings.Contains(out, "outbound") { + t.Errorf("warning should name the configured direction:\n%s", out) + } + if !strings.Contains(out, "inbound") { + t.Errorf("warning should report the declared direction:\n%s", out) + } +} + +// The correct placement, a both-chain plugin, and an unknown name must +// all stay silent. The unknown-name case matters because Build already +// reports it with a better error listing every registered plugin. +func TestWarnPluginDirectionsStaysSilent(t *testing.T) { + cases := []struct { + name string + needs string + inbound, outbnd []string + }{ + {"correct placement", "jwt-validation", []string{"jwt-validation"}, nil}, + {"both-chain plugin inbound", "opa", []string{"opa"}, nil}, + {"both-chain plugin outbound", "opa", nil, []string{"opa"}}, + {"unknown plugin name", "", nil, []string{"no-such-plugin"}}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if c.needs != "" { + if _, ok := factoryFor(c.needs); !ok { + t.Skipf("%s not linked into this test binary", c.needs) + } + } + logger, buf := warnBuffer() + WarnPluginDirections(cfgWith(c.inbound, c.outbnd), logger) + if out := buf.String(); out != "" { + t.Errorf("expected no warning, got:\n%s", out) + } + }) + } +} + +// The warning is advisory: a misplaced plugin must still build into a +// working pipeline. If this ever fails, the check has become fatal and +// the feature's core promise is broken. +func TestWarnPluginDirectionsIsAdvisory(t *testing.T) { + if _, ok := factoryFor("mcp-parser"); !ok { + t.Skip("mcp-parser not linked into this test binary") + } + logger, buf := warnBuffer() + // mcp-parser declares outbound; build it inbound anyway. + entries := []config.PluginEntry{{Name: "mcp-parser"}} + WarnPluginDirections(cfgWith([]string{"mcp-parser"}, nil), logger) + if buf.String() == "" { + t.Fatal("setup: expected a warning for the misplaced plugin") + } + p, err := Build(entries) + if err != nil { + t.Fatalf("Build must succeed despite the direction warning: %v", err) + } + if p == nil { + t.Fatal("Build returned a nil pipeline") + } +} + +// A nil config must not panic — main() calls this right after Validate, +// but defensive since the sibling WarnEmptyPipelines is nil-tolerant too. +func TestWarnPluginDirectionsNilConfig(t *testing.T) { + logger, buf := warnBuffer() + WarnPluginDirections(nil, logger) + if out := buf.String(); out != "" { + t.Errorf("nil config should warn nothing, got:\n%s", out) + } +} diff --git a/authbridge/authlib/plugins/ibac/plugin.go b/authbridge/authlib/plugins/ibac/plugin.go index 17fb135f5..0630cd872 100644 --- a/authbridge/authlib/plugins/ibac/plugin.go +++ b/authbridge/authlib/plugins/ibac/plugin.go @@ -280,6 +280,7 @@ func (p *IBAC) Name() string { return "ibac" } func (p *IBAC) Capabilities() pipeline.PluginCapabilities { return pipeline.PluginCapabilities{ + Directions: []pipeline.Direction{pipeline.Outbound}, // At least one outbound protocol parser must run before IBAC. // IBAC is a defense-in-depth layer that only fires on traffic // a parser classified — without a parser, IBAC has no way to diff --git a/authbridge/authlib/plugins/inferenceparser/plugin.go b/authbridge/authlib/plugins/inferenceparser/plugin.go index 5c80afd31..854640c3d 100644 --- a/authbridge/authlib/plugins/inferenceparser/plugin.go +++ b/authbridge/authlib/plugins/inferenceparser/plugin.go @@ -26,6 +26,7 @@ func (p *InferenceParser) Name() string { return "inference-parser" } func (p *InferenceParser) Capabilities() pipeline.PluginCapabilities { return pipeline.PluginCapabilities{ + Directions: []pipeline.Direction{pipeline.Outbound}, ReadsBody: true, Description: "Parses LLM completions into pctx.Extensions.Inference.", } diff --git a/authbridge/authlib/plugins/jwtvalidation/plugin.go b/authbridge/authlib/plugins/jwtvalidation/plugin.go index da49d7c0b..646fad9f3 100644 --- a/authbridge/authlib/plugins/jwtvalidation/plugin.go +++ b/authbridge/authlib/plugins/jwtvalidation/plugin.go @@ -222,6 +222,7 @@ func (p *JWTValidation) Name() string { return "jwt-validation" } func (p *JWTValidation) Capabilities() pipeline.PluginCapabilities { return pipeline.PluginCapabilities{ + Directions: []pipeline.Direction{pipeline.Inbound}, Description: "Inbound JWT validation (signature, issuer, audience) against JWKS.", } } diff --git a/authbridge/authlib/plugins/litellm_budgettrack/plugin.go b/authbridge/authlib/plugins/litellm_budgettrack/plugin.go index 22c8d886e..140348f1c 100644 --- a/authbridge/authlib/plugins/litellm_budgettrack/plugin.go +++ b/authbridge/authlib/plugins/litellm_budgettrack/plugin.go @@ -119,6 +119,7 @@ func (p *BudgetTrack) Name() string { return "litellm-budget-track" } func (p *BudgetTrack) Capabilities() pipeline.PluginCapabilities { return pipeline.PluginCapabilities{ + Directions: []pipeline.Direction{pipeline.Inbound}, // ReadsBody: the plugin parses the response body (streamed usage). It // makes Pipeline.NeedsBody() true so the extproc (envoy-sidecar) listener // buffers the response body and takes its body-phase branch; without it @@ -131,6 +132,12 @@ func (p *BudgetTrack) Capabilities() pipeline.PluginCapabilities { } } +// ConfigSchema implements pipeline.SchemaProvider; surfaces field +// metadata to abctl edit templates and other config-aware tooling. +func (p *BudgetTrack) ConfigSchema() []pipeline.FieldSchema { + return pipeline.SchemaOf(budgetTrackConfig{}) +} + func (p *BudgetTrack) Configure(raw json.RawMessage) error { if err := json.Unmarshal(raw, &p.cfg); err != nil { return fmt.Errorf("litellm-budget-track config: %w", err) diff --git a/authbridge/authlib/plugins/mcpparser/plugin.go b/authbridge/authlib/plugins/mcpparser/plugin.go index bb409a98b..92e4a02b9 100644 --- a/authbridge/authlib/plugins/mcpparser/plugin.go +++ b/authbridge/authlib/plugins/mcpparser/plugin.go @@ -44,7 +44,7 @@ type mcpConfig struct { // Path-shape detection only fires on body-less requests; body- // having JSON-RPC requests are parsed regardless of path (the // JSON-RPC body itself is the protocol signal). - Paths []string `json:"paths"` + Paths []string `json:"paths" description:"URL path globs treated as MCP endpoints for body-less request detection." default:"[\"/mcp\"]"` } func (c *mcpConfig) applyDefaults() { @@ -88,11 +88,18 @@ func (p *MCPParser) Name() string { return "mcp-parser" } func (p *MCPParser) Capabilities() pipeline.PluginCapabilities { return pipeline.PluginCapabilities{ + Directions: []pipeline.Direction{pipeline.Outbound}, ReadsBody: true, Description: "Parses MCP tool calls/results into pctx.Extensions.MCP.", } } +// ConfigSchema implements pipeline.SchemaProvider; surfaces field +// metadata to abctl edit templates and other config-aware tooling. +func (p *MCPParser) ConfigSchema() []pipeline.FieldSchema { + return pipeline.SchemaOf(mcpConfig{}) +} + // Configure decodes the optional `paths` list and compiles a path // matcher used by body-less transport-layer detection. Always // initializes the matcher (default paths are applied when omitted) diff --git a/authbridge/authlib/plugins/opa/plugin.go b/authbridge/authlib/plugins/opa/plugin.go index f790f282b..49810b3e7 100644 --- a/authbridge/authlib/plugins/opa/plugin.go +++ b/authbridge/authlib/plugins/opa/plugin.go @@ -29,13 +29,29 @@ const ( pathOutboundResponse = "authbridge/outbound/response" ) +// opaConfig is the plugin's local config schema. +// +// Field tags drive both runtime decoding (json) and operator-facing +// schema introspection (description / required / default / enum). +// See pipeline/schema.go for the consumer contract. type opaConfig struct { - BundleURL string `json:"bundle_url"` - AgentIDFile string `json:"agent_id_file"` - AgentID string `json:"agent_id"` - PollingMinDelay int `json:"polling_min_delay"` - PollingMaxDelay int `json:"polling_max_delay"` - Include []string `json:"include"` + BundleURL string `json:"bundle_url" required:"true" description:"Base URL of the Rossoctl Bundle Server (HTTP, in-cluster)."` + + // AgentIDFile / AgentID are a soft either-or: when both are empty + // AgentIDFile defaults to the Rossoctl convention below. A missing + // file only WARNs — Init polls for it in the background — so + // neither field is `required`. + AgentIDFile string `json:"agent_id_file" description:"Path to the file holding the agent's client ID. Defaults only when agent_id is also empty." default:"/shared/client-id.txt"` + AgentID string `json:"agent_id" description:"Inline agent ID. When set, agent_id_file is ignored."` + + PollingMinDelay int `json:"polling_min_delay" description:"Minimum bundle polling interval in seconds." default:"10"` + PollingMaxDelay int `json:"polling_max_delay" description:"Maximum bundle polling interval in seconds." default:"120"` + + // Include names optional field groups to add to the OPA input + // document. newIncludeSet always enables mcp.params.name and + // mcp.params.uri regardless of this list, so the effective default + // is not simply empty. + Include []string `json:"include" description:"Optional input field groups to expose to policy. mcp.params.name and mcp.params.uri are always included."` } // includeSet is built once at Configure time from the Include config list. @@ -190,10 +206,17 @@ func (p *OPA) Name() string { return "opa" } func (p *OPA) Capabilities() pipeline.PluginCapabilities { return pipeline.PluginCapabilities{ + Directions: []pipeline.Direction{pipeline.Inbound, pipeline.Outbound}, Description: "OPA policy enforcement for inbound and outbound requests.", } } +// ConfigSchema implements pipeline.SchemaProvider; surfaces field +// metadata to abctl edit templates and other config-aware tooling. +func (p *OPA) ConfigSchema() []pipeline.FieldSchema { + return pipeline.SchemaOf(opaConfig{}) +} + func (p *OPA) Configure(raw json.RawMessage) error { var c opaConfig if len(raw) > 0 { diff --git a/authbridge/authlib/plugins/registry.go b/authbridge/authlib/plugins/registry.go index fc4cc0a8f..91d6f338f 100644 --- a/authbridge/authlib/plugins/registry.go +++ b/authbridge/authlib/plugins/registry.go @@ -214,6 +214,7 @@ func cloneCatalog(in []CatalogEntry) []CatalogEntry { Description: caps.Description, Requires: append([]string(nil), caps.Requires...), RequiresAny: append([]string(nil), caps.RequiresAny...), + Directions: append([]pipeline.Direction(nil), caps.Directions...), }, Fields: cloneFieldSchemas(in[i].Fields), } diff --git a/authbridge/authlib/plugins/sessionbudget/plugin.go b/authbridge/authlib/plugins/sessionbudget/plugin.go index 57d4b1d87..19e2fe9e9 100644 --- a/authbridge/authlib/plugins/sessionbudget/plugin.go +++ b/authbridge/authlib/plugins/sessionbudget/plugin.go @@ -118,10 +118,17 @@ func (p *SessionBudget) Name() string { return "session-budget" } func (p *SessionBudget) Capabilities() pipeline.PluginCapabilities { return pipeline.PluginCapabilities{ + Directions: []pipeline.Direction{pipeline.Outbound}, Description: "Enforce per-session token, call, and duration budgets via Redis.", } } +// ConfigSchema implements pipeline.SchemaProvider; surfaces field +// metadata to abctl edit templates and other config-aware tooling. +func (p *SessionBudget) ConfigSchema() []pipeline.FieldSchema { + return pipeline.SchemaOf(config{}) +} + func (p *SessionBudget) Configure(raw json.RawMessage) error { p.cfg = config{ OnExceed: "deny", diff --git a/authbridge/authlib/plugins/sparc/plugin.go b/authbridge/authlib/plugins/sparc/plugin.go index f0a4ebd20..278e13812 100644 --- a/authbridge/authlib/plugins/sparc/plugin.go +++ b/authbridge/authlib/plugins/sparc/plugin.go @@ -205,6 +205,7 @@ func (p *SPARC) Name() string { return "sparc" } func (p *SPARC) Capabilities() pipeline.PluginCapabilities { return pipeline.PluginCapabilities{ + Directions: []pipeline.Direction{pipeline.Outbound}, // Needs a parser to supply SPARC's inputs. inference-parser provides the // conversation + tool specs (both modes); mcp-parser provides the tool // call (mcp mode). RequiresAny is a static "at least one" check; the diff --git a/authbridge/authlib/plugins/staticinject/plugin.go b/authbridge/authlib/plugins/staticinject/plugin.go index 7a28a0e29..85533b2c6 100644 --- a/authbridge/authlib/plugins/staticinject/plugin.go +++ b/authbridge/authlib/plugins/staticinject/plugin.go @@ -141,6 +141,7 @@ func (p *StaticInject) Name() string { return "static-inject" } func (p *StaticInject) Capabilities() pipeline.PluginCapabilities { return pipeline.PluginCapabilities{ + Directions: []pipeline.Direction{pipeline.Outbound}, Description: "Swaps a placeholder credential for a real static credential on outbound requests.", } } diff --git a/authbridge/authlib/plugins/tokenbroker/plugin.go b/authbridge/authlib/plugins/tokenbroker/plugin.go index 6b6fd884c..ac08ae8eb 100644 --- a/authbridge/authlib/plugins/tokenbroker/plugin.go +++ b/authbridge/authlib/plugins/tokenbroker/plugin.go @@ -194,6 +194,7 @@ func (p *TokenBroker) Name() string { return "token-broker" } func (p *TokenBroker) Capabilities() pipeline.PluginCapabilities { return pipeline.PluginCapabilities{ + Directions: []pipeline.Direction{pipeline.Outbound}, Description: "Token broker: exchanges incoming tokens against the configured IdP.", } } diff --git a/authbridge/authlib/plugins/tokenexchange/plugin.go b/authbridge/authlib/plugins/tokenexchange/plugin.go index be875b770..b0495aef1 100644 --- a/authbridge/authlib/plugins/tokenexchange/plugin.go +++ b/authbridge/authlib/plugins/tokenexchange/plugin.go @@ -357,6 +357,7 @@ func (p *TokenExchange) Name() string { return "token-exchange" } func (p *TokenExchange) Capabilities() pipeline.PluginCapabilities { return pipeline.PluginCapabilities{ + Directions: []pipeline.Direction{pipeline.Outbound}, Description: "RFC 8693 outbound token exchange per route. Supports Keycloak, Entra ID, Okta, and any RFC 8693-compliant IdP.", } } diff --git a/authbridge/authlib/plugins/warn.go b/authbridge/authlib/plugins/warn.go new file mode 100644 index 000000000..d9a591863 --- /dev/null +++ b/authbridge/authlib/plugins/warn.go @@ -0,0 +1,89 @@ +package plugins + +import ( + "log/slog" + + "github.com/rossoctl/cortex/authbridge/authlib/config" + "github.com/rossoctl/cortex/authbridge/authlib/pipeline" +) + +// WarnPluginDirections emits a startup WARN for every configured plugin +// that declares a set of pipeline directions (Capabilities().Directions) +// not including the chain it was actually placed in — e.g. jwt-validation +// (inbound-only) configured under `outbound:`. +// +// Advisory, never fatal. No plugin enforces direction at runtime, so a +// misplaced plugin is a probable misconfiguration rather than a +// guaranteed one: it typically runs as dead code (a validator that never +// sees a token, a parser whose protocol never appears on that side). +// Failing the boot would break configurations that work today, so this +// only makes the condition visible in logs — matching the precedent set +// by config.WarnEmptyPipelines, which treats an open proxy the same way. +// +// Plugins that declare no Directions are unconstrained and never warn +// (see PluginCapabilities.Supports). Unknown plugin names are skipped +// silently: plugins.Build already fails on those with a much better +// error listing every registered name, and duplicating it here would +// double up the diagnostic. +// +// This function lives in authlib/plugins rather than beside +// WarnEmptyPipelines in authlib/config because it needs the plugin +// catalog: plugins already imports config, so the reverse would be an +// import cycle. +// +// Call this from each cmd entry point AFTER Validate succeeds and BEFORE +// the pipelines are built, alongside config.WarnEmptyPipelines. Pass +// slog.Default() unless you need a scoped logger. +func WarnPluginDirections(cfg *config.Config, logger *slog.Logger) { + if cfg == nil { + return + } + if logger == nil { + logger = slog.Default() + } + byName := make(map[string]pipeline.PluginCapabilities) + for _, e := range Catalog() { + byName[e.Name] = e.Capabilities + } + warnChain(cfg.Pipeline.Inbound.Plugins, pipeline.Inbound, byName, logger) + warnChain(cfg.Pipeline.Outbound.Plugins, pipeline.Outbound, byName, logger) +} + +// warnChain checks one chain's entries against their declared directions. +func warnChain( + entries []config.PluginEntry, + dir pipeline.Direction, + byName map[string]pipeline.PluginCapabilities, + logger *slog.Logger, +) { + for i, e := range entries { + caps, known := byName[e.Name] + if !known { + continue // Build reports unknown names with a better error. + } + if caps.Supports(dir) { + continue + } + logger.Warn("plugin is configured in a pipeline direction it does not declare support for; "+ + "it will run but is likely misplaced", + "plugin", e.Name, + "configured_direction", dir.String(), + "declared_directions", directionNames(caps.Directions), + "position", i+1, + ) + } +} + +// directionNames renders a Direction slice as strings for log output. +// Keeps the WARN readable ("[inbound]") instead of printing the +// underlying enum ints. +func directionNames(ds []pipeline.Direction) []string { + if len(ds) == 0 { + return nil + } + out := make([]string, len(ds)) + for i, d := range ds { + out[i] = d.String() + } + return out +} diff --git a/authbridge/authlib/sessionapi/catalog_adapter.go b/authbridge/authlib/sessionapi/catalog_adapter.go index f0b64e52a..f2cbc47bc 100644 --- a/authbridge/authlib/sessionapi/catalog_adapter.go +++ b/authbridge/authlib/sessionapi/catalog_adapter.go @@ -10,10 +10,15 @@ import ( // proxy, -envoy) plug it in identically; centralizing the conversion // here keeps the field list one-place. // -// Direction is left empty: the catalog describes plugin TYPES, and -// most plugins can be configured into either chain (parsers especially). -// abctl renders direction only for the active pipeline, where the -// answer is positional, not type-level. +// The singular Direction is left empty: it means "the chain this +// configured instance sits in", which is a property of an instance, not +// of a type. abctl renders it only for the active pipeline, where the +// answer is positional. +// +// The plural Directions IS populated: it is the type-level declaration +// of which chains a plugin supports (PluginCapabilities.Directions), +// which is exactly what a config generator needs to place a plugin. +// Empty means the plugin makes no claim. // // Fields is populated for plugins that implement // pipeline.SchemaProvider (most config-bearing plugins). Plugins @@ -26,6 +31,7 @@ func PluginsCatalog() []CatalogEntry { n := e.Capabilities.Normalize() out[i] = CatalogEntry{ Name: e.Name, + Directions: directionStrings(n.Directions), ReadsBody: n.ReadsBody, Requires: n.Requires, RequiresAny: n.RequiresAny, @@ -58,3 +64,25 @@ func convertFieldSchemas(in []pipeline.FieldSchema) []FieldSchemaEntry { } return out } + +// directionStrings renders a Direction slice as its wire form. +// +// The wire type is []string rather than []pipeline.Direction on purpose: +// Direction.UnmarshalJSON decodes any unrecognized string to Inbound +// without erroring (a deliberate forward-compatibility choice for the +// single-valued field), which on a slice would silently turn a future +// third direction into a false "inbound" claim. Strings keep the wire +// honest and match the existing Requires/RequiresAny precedent. +// +// Returns nil for empty input so the field elides via omitempty and +// "unconstrained" stays absent rather than an empty array. +func directionStrings(ds []pipeline.Direction) []string { + if len(ds) == 0 { + return nil + } + out := make([]string, len(ds)) + for i, d := range ds { + out[i] = d.String() + } + return out +} diff --git a/authbridge/authlib/sessionapi/server.go b/authbridge/authlib/sessionapi/server.go index 1f45aad2a..a6df25b33 100644 --- a/authbridge/authlib/sessionapi/server.go +++ b/authbridge/authlib/sessionapi/server.go @@ -53,8 +53,16 @@ type Server struct { // that documents bodyAccess as deprecated, so there's no compat cost to // emit the right name from day one. type CatalogEntry struct { - Name string `json:"name"` - Direction string `json:"direction,omitempty"` + Name string `json:"name"` + // Direction is the POSITIONAL chain of one configured instance. The + // catalog describes plugin types, not instances, so PluginsCatalog + // leaves it empty; see Directions for the type-level answer. + Direction string `json:"direction,omitempty"` + // Directions lists the chains this plugin TYPE declares support for + // ("inbound" / "outbound"). Empty means unconstrained. Distinct from + // Direction above: this is what the plugin supports, not where one + // instance sits. Config generators read this to place a plugin. + Directions []string `json:"directions,omitempty"` ReadsBody bool `json:"readsBody,omitempty"` Requires []string `json:"requires,omitempty"` RequiresAny []string `json:"requiresAny,omitempty"` @@ -163,6 +171,7 @@ func (s *Server) handleHealthz(w http.ResponseWriter, _ *http.Request) { type pipelinePluginView struct { Name string `json:"name"` Direction string `json:"direction"` + Directions []string `json:"directions,omitempty"` Position int `json:"position"` // 1-based order within its direction ReadsBody bool `json:"readsBody"` Requires []string `json:"requires,omitempty"` @@ -225,6 +234,7 @@ func describePipeline(h *pipeline.Holder, direction string) []pipelinePluginView view := pipelinePluginView{ Name: pl.Name(), Direction: direction, + Directions: directionStrings(caps.Directions), Position: i + 1, ReadsBody: caps.ReadsBody, Requires: caps.Requires, diff --git a/authbridge/cmd/abctl/README.md b/authbridge/cmd/abctl/README.md index b66694734..e8d4b6a25 100644 --- a/authbridge/cmd/abctl/README.md +++ b/authbridge/cmd/abctl/README.md @@ -89,7 +89,8 @@ The UI has these top-level panes. `Enter` drills in; `Esc` backs out. Lists every plugin the running binary knows how to construct, including ones not in the active pipeline. Useful for discovering what's available before adding to the pipeline. Sourced from - `/v1/plugins`. + `/v1/plugins`, which also reports each plugin's supported chain(s) + as `directions`. ## Keybindings @@ -130,6 +131,10 @@ and asks `apply this change? (y/N)`. Confirming runs edit), then polls the framework's `/reload/status` until the reload completes (success or failure). +Each template block in the reference carries a `# chain:` line naming +the pipeline(s) that plugin declares support for, so you can tell where +to paste it before you do. Plugins that declare nothing get no line. + The single edit flow covers four operations: - **Edit a value** — change a config field of an existing plugin - **Reorder** — move a plugin's lines up or down @@ -160,6 +165,21 @@ The y/N prompt becomes "apply anyway? (y/N)" — abctl's check is non-blocking. The framework's own validateRelationships is the source of truth and will fire again at reload regardless. +Advisories are reported separately, under their own banner, because +the framework will *accept* them: + +```text +1 advisory — reload will accept, but check: + • [outbound] jwt-validation pos 2: declared for inbound; likely belongs in the inbound chain +``` + +Today the only advisory is a plugin placed in a chain it doesn't +declare support for (the `directions` field on `/v1/plugins`). Such a +plugin runs, typically as dead code — an inbound validator that never +sees a token, a parser whose protocol never appears on that side. The +agent logs the same mismatch as a startup WARN. Advisories alone leave +the prompt as "apply this change? (y/N)": there is nothing to override. + Validation is silently skipped when the catalog isn't loaded (operator hasn't pressed `P` yet). Visit the catalog pane once to populate it for the rest of the session. diff --git a/authbridge/cmd/abctl/apiclient/client.go b/authbridge/cmd/abctl/apiclient/client.go index 15a4e2be0..14d5de61c 100644 --- a/authbridge/cmd/abctl/apiclient/client.go +++ b/authbridge/cmd/abctl/apiclient/client.go @@ -85,6 +85,7 @@ type PipelineView struct { type PipelinePlugin struct { Name string `json:"name"` Direction string `json:"direction"` + Directions []string `json:"directions,omitempty"` Position int `json:"position"` ReadsBody bool `json:"readsBody"` Requires []string `json:"requires,omitempty"` @@ -111,8 +112,12 @@ type PluginCatalog struct { // Describes a registered plugin's static type-level metadata; the // catalog includes plugins not currently in the active pipeline. type PluginCatalogEntry struct { - Name string `json:"name"` + Name string `json:"name"` + // Direction is positional (which chain one configured instance sits + // in) and is empty for catalog entries. Directions below is the + // type-level set of chains the plugin declares support for. Direction string `json:"direction,omitempty"` + Directions []string `json:"directions,omitempty"` ReadsBody bool `json:"readsBody,omitempty"` Requires []string `json:"requires,omitempty"` RequiresAny []string `json:"requiresAny,omitempty"` diff --git a/authbridge/cmd/abctl/apiclient/client_test.go b/authbridge/cmd/abctl/apiclient/client_test.go index 4553855e2..667752cd3 100644 --- a/authbridge/cmd/abctl/apiclient/client_test.go +++ b/authbridge/cmd/abctl/apiclient/client_test.go @@ -292,3 +292,55 @@ func TestPipelinePluginDecodesCapabilityMetadata(t *testing.T) { t.Errorf("Description = %q", p.Description) } } + +// TestGetPluginCatalog_DecodesDirections guards the same server/client +// tag boundary for the type-level `directions` field. The singular +// `direction` (positional, empty in the catalog) and the plural +// `directions` (which chains the plugin type supports) are separate +// keys with separate meanings; decoding one into the other would make +// abctl's chain hints and its advisory validation silently wrong. +func TestGetPluginCatalog_DecodesDirections(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/v1/plugins" { + http.NotFound(w, r) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{ + "plugins": [ + {"name": "jwt-validation", "directions": ["inbound"]}, + {"name": "opa", "directions": ["inbound", "outbound"]}, + {"name": "unconstrained"} + ] + }`)) + })) + defer ts.Close() + + cat, err := New(ts.URL).GetPluginCatalog(context.Background()) + if err != nil { + t.Fatalf("GetPluginCatalog: %v", err) + } + if len(cat.Plugins) != 3 { + t.Fatalf("got %d plugins, want 3", len(cat.Plugins)) + } + + byName := map[string][]string{} + for _, p := range cat.Plugins { + byName[p.Name] = p.Directions + // The positional field must stay empty for catalog entries — the + // server deliberately omits it there. + if p.Direction != "" { + t.Errorf("%s: singular Direction should be empty for a catalog entry, got %q", + p.Name, p.Direction) + } + } + if got := byName["jwt-validation"]; len(got) != 1 || got[0] != "inbound" { + t.Errorf(`jwt-validation directions = %v, want ["inbound"]`, got) + } + if got := byName["opa"]; len(got) != 2 || got[0] != "inbound" || got[1] != "outbound" { + t.Errorf(`opa directions = %v, want ["inbound","outbound"]`, got) + } + if got := byName["unconstrained"]; len(got) != 0 { + t.Errorf("a plugin with no directions key should decode to empty, got %v", got) + } +} diff --git a/authbridge/cmd/abctl/edit/templates.go b/authbridge/cmd/abctl/edit/templates.go index 38a16ec0d..ab198bdf6 100644 --- a/authbridge/cmd/abctl/edit/templates.go +++ b/authbridge/cmd/abctl/edit/templates.go @@ -87,6 +87,17 @@ func renderPluginTemplate(b *strings.Builder, p apiclient.PluginCatalogEntry) { b.WriteString(p.Description) b.WriteString("\n") } + // Which chain this template belongs in. Emitted only when the plugin + // declares it — an unconstrained plugin gets no line rather than a + // misleading "chain: any", so silence keeps its existing meaning. + // This is the whole point of the catalog's directions field: the + // reference tells the operator WHERE to paste the block, not just + // what it does. + if len(p.Directions) > 0 { + b.WriteString("# chain: ") + b.WriteString(strings.Join(p.Directions, ", ")) + b.WriteString("\n") + } // Split top-level fields into required vs optional for ordering // (required render first inside the config: block). Object fields @@ -372,6 +383,10 @@ func placeholderFor(f apiclient.PluginFieldEntry) string { return `""` case "int": return "0" + case "number": + // Float-typed field (per-token cost, budget). "0" is valid YAML + // for a float, and avoids implying a fixed precision. + return "0" case "bool": return "false" case "[]string": diff --git a/authbridge/cmd/abctl/edit/templates_test.go b/authbridge/cmd/abctl/edit/templates_test.go index e81c70c7d..313c5fc50 100644 --- a/authbridge/cmd/abctl/edit/templates_test.go +++ b/authbridge/cmd/abctl/edit/templates_test.go @@ -40,7 +40,7 @@ func TestRenderTemplates_PluginWithFields(t *testing.T) { {Name: "timeout_ms", Type: "int", Default: "5000", Description: "Per-call timeout."}, {Name: "unclassified_policy", Type: "string", Default: "passthrough", - Enum: []string{"passthrough", "judge"}, + Enum: []string{"passthrough", "judge"}, Description: "Behavior when no parser claimed the request."}, }, }, @@ -432,3 +432,52 @@ func TestFetchCmd_NoTemplatesWhenCatalogNil(t *testing.T) { t.Fatalf("tempfile should not contain fence marker when catalog is nil:\n%s", string(body)) } } + +// The rendered reference tells operators WHICH chain a plugin belongs +// in — the direct payoff of the catalog's directions field, since the +// templates block is where a plugin gets copied from. +func TestRenderTemplates_ChainAnnotation(t *testing.T) { + out := string(RenderTemplates([]apiclient.PluginCatalogEntry{ + {Name: "jwt-validation", Description: "Inbound JWT.", Directions: []string{"inbound"}}, + {Name: "opa", Description: "Policy.", Directions: []string{"inbound", "outbound"}}, + {Name: "legacy", Description: "Declares nothing."}, + })) + + if !strings.Contains(out, "# chain: inbound\n") { + t.Errorf("single-direction plugin should get a chain line:\n%s", out) + } + if !strings.Contains(out, "# chain: inbound, outbound\n") { + t.Errorf("both-chain plugin should list both:\n%s", out) + } + // A plugin declaring nothing gets NO chain line — silence keeps its + // existing meaning rather than asserting a misleading "any". + legacy := out[strings.Index(out, "--- legacy ---"):] + if strings.Contains(legacy, "# chain:") { + t.Errorf("unconstrained plugin should get no chain line:\n%s", legacy) + } + // Everything stays a comment; the block must not become live YAML. + for _, ln := range strings.Split(out, "\n") { + if strings.Contains(ln, "chain:") && !strings.HasPrefix(strings.TrimSpace(ln), "#") { + t.Errorf("chain annotation must be commented, got %q", ln) + } + } +} + +// A float-typed field renders a numeric placeholder rather than falling +// through to the quoted-empty-string default. +func TestRenderTemplates_NumberPlaceholder(t *testing.T) { + out := string(RenderTemplates([]apiclient.PluginCatalogEntry{{ + Name: "litellm-budget-track", + Directions: []string{"inbound"}, + Fields: []apiclient.PluginFieldEntry{ + {Name: "max_budget", Type: "number", Required: true}, + {Name: "spend_file", Type: "string", Required: true}, + }, + }})) + if !strings.Contains(out, "max_budget: 0") { + t.Errorf("number field should render a numeric placeholder:\n%s", out) + } + if strings.Contains(out, `max_budget: ""`) { + t.Errorf("number field must not render a quoted empty string:\n%s", out) + } +} diff --git a/authbridge/cmd/abctl/edit/validate.go b/authbridge/cmd/abctl/edit/validate.go index 0b162b29e..8078fa007 100644 --- a/authbridge/cmd/abctl/edit/validate.go +++ b/authbridge/cmd/abctl/edit/validate.go @@ -2,12 +2,31 @@ package edit import ( "fmt" + "slices" + "strings" "gopkg.in/yaml.v3" "github.com/rossoctl/cortex/authbridge/cmd/abctl/apiclient" ) +// Severity distinguishes a problem the framework will reject from one +// that is merely suspicious. The zero value is SeverityError so every +// existing construction site keeps its original meaning. +type Severity int + +const ( + // SeverityError: the framework's reload will reject this pipeline. + // Unmet Requires, misordered dependencies, unknown plugin names. + SeverityError Severity = iota + // SeverityWarning: the framework will ACCEPT this pipeline, but it + // looks wrong. Today the only case is a plugin placed in a chain it + // does not declare support for — it runs, typically as dead code. + // Rendered separately from errors so the "reload will reject" + // banner stays truthful. + SeverityWarning +) + // ValidationError describes one problem with a proposed pipeline, // detected by abctl before kubectl apply. The framework's own // validateRelationships is the source of truth (and runs again after @@ -15,6 +34,9 @@ import ( type ValidationError struct { // Direction is "inbound" or "outbound". Direction string + // Severity reports whether the framework will reject this (the + // default) or merely that it looks misconfigured. + Severity Severity // PluginName is the offending plugin's name. PluginName string // Position is the offending plugin's 1-based position in its chain. @@ -102,6 +124,22 @@ func validateChain(direction string, chain pipelineChain, byName map[string]apic continue } + // Direction: advisory only. The framework accepts a plugin in + // either chain (nothing enforces direction at runtime), so a + // mismatch is a probable misconfiguration, not a rejection — + // hence SeverityWarning. An entry declaring no directions is + // unconstrained and never flagged. + if len(entry.Directions) > 0 && !slices.Contains(entry.Directions, direction) { + errs = append(errs, ValidationError{ + Direction: direction, + Severity: SeverityWarning, + PluginName: p.Name, + Position: pos, + Message: fmt.Sprintf("declared for %s; likely belongs in the %s chain", + strings.Join(entry.Directions, "/"), strings.Join(entry.Directions, " or ")), + }) + } + // Requires: every name MUST appear at strictly-lower position. for _, req := range entry.Requires { rp, present := positions[req] diff --git a/authbridge/cmd/abctl/edit/validate_test.go b/authbridge/cmd/abctl/edit/validate_test.go index fe1aef393..633ac1a65 100644 --- a/authbridge/cmd/abctl/edit/validate_test.go +++ b/authbridge/cmd/abctl/edit/validate_test.go @@ -106,3 +106,114 @@ func TestValidatePipeline_NilCatalogSkips(t *testing.T) { t.Fatalf("nil catalog should disable validation, got %+v", errs) } } + +// --- direction advisories ------------------------------------------------- + +// directionFixtureCatalog declares Directions, unlike validateFixtureCatalog +// (whose entries are all unconstrained — which is itself the back-compat +// case: a catalog from an older agent yields no advisories at all). +func directionFixtureCatalog() []apiclient.PluginCatalogEntry { + return []apiclient.PluginCatalogEntry{ + {Name: "jwt-validation", Directions: []string{"inbound"}}, + {Name: "token-exchange", Directions: []string{"outbound"}}, + {Name: "opa", Directions: []string{"inbound", "outbound"}}, + {Name: "mcp-parser", Directions: []string{"outbound"}}, + {Name: "ibac", Directions: []string{"outbound"}, Requires: []string{"mcp-parser"}}, + {Name: "legacy", Description: "declares nothing"}, + } +} + +func TestValidatePipeline_DirectionMismatchIsAdvisory(t *testing.T) { + // jwt-validation is inbound-only, placed outbound. + subtree := []byte("pipeline:\n outbound:\n plugins:\n - name: jwt-validation\n") + errs := ValidatePipeline(subtree, directionFixtureCatalog()) + if len(errs) != 1 { + t.Fatalf("want exactly 1 issue, got %d: %+v", len(errs), errs) + } + ve := errs[0] + if ve.Severity != SeverityWarning { + t.Errorf("direction mismatch should be SeverityWarning, got %v", ve.Severity) + } + if ve.PluginName != "jwt-validation" || ve.Direction != "outbound" || ve.Position != 1 { + t.Errorf("unexpected issue shape: %+v", ve) + } + if !strings.Contains(ve.Message, "inbound") { + t.Errorf("message should name the declared direction, got %q", ve.Message) + } +} + +// Correct placement, a both-chain plugin, and an entry declaring nothing +// must all produce no advisory. +func TestValidatePipeline_DirectionNoFalsePositives(t *testing.T) { + cases := []struct { + name string + subtree string + }{ + {"correct inbound", "pipeline:\n inbound:\n plugins:\n - name: jwt-validation\n"}, + {"correct outbound", "pipeline:\n outbound:\n plugins:\n - name: token-exchange\n"}, + {"both-chain inbound", "pipeline:\n inbound:\n plugins:\n - name: opa\n"}, + {"both-chain outbound", "pipeline:\n outbound:\n plugins:\n - name: opa\n"}, + {"unconstrained entry", "pipeline:\n inbound:\n plugins:\n - name: legacy\n"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + errs := ValidatePipeline([]byte(c.subtree), directionFixtureCatalog()) + if len(errs) != 0 { + t.Errorf("expected no issues, got %+v", errs) + } + }) + } +} + +// A hard dependency error and a direction advisory in the same edit must +// keep distinct severities: the overlay renders them under different +// banners, and only the error justifies the "reload will reject" claim. +func TestValidatePipeline_SeveritiesCoexist(t *testing.T) { + // ibac Requires mcp-parser (absent -> error); jwt-validation is + // misplaced outbound (-> warning). + subtree := []byte("pipeline:\n outbound:\n plugins:\n" + + " - name: ibac\n - name: jwt-validation\n") + errs := ValidatePipeline(subtree, directionFixtureCatalog()) + + var nErr, nWarn int + for _, ve := range errs { + switch ve.Severity { + case SeverityWarning: + nWarn++ + if ve.PluginName != "jwt-validation" { + t.Errorf("warning should be for jwt-validation, got %q", ve.PluginName) + } + case SeverityError: + nErr++ + if ve.PluginName != "ibac" { + t.Errorf("error should be for ibac, got %q", ve.PluginName) + } + } + } + if nErr != 1 || nWarn != 1 { + t.Fatalf("want 1 error + 1 warning, got %d + %d: %+v", nErr, nWarn, errs) + } +} + +// Every pre-existing check keeps SeverityError (the zero value), so the +// "framework reload will reject" banner stays accurate for them. +func TestValidatePipeline_ExistingChecksAreErrors(t *testing.T) { + cases := map[string]string{ + "unmet requires": "pipeline:\n outbound:\n plugins:\n - name: ibac\n", + "unknown name": "pipeline:\n inbound:\n plugins:\n - name: nope\n", + "misordered": "pipeline:\n outbound:\n plugins:\n - name: ibac\n - name: mcp-parser\n", + } + for name, subtree := range cases { + t.Run(name, func(t *testing.T) { + errs := ValidatePipeline([]byte(subtree), directionFixtureCatalog()) + if len(errs) == 0 { + t.Fatal("expected at least one issue") + } + for _, ve := range errs { + if ve.Severity != SeverityError { + t.Errorf("%s should be SeverityError, got %v (%s)", name, ve.Severity, ve.Message) + } + } + }) + } +} diff --git a/authbridge/cmd/abctl/tui/catalog_pane.go b/authbridge/cmd/abctl/tui/catalog_pane.go index 6eb656881..174ac30c5 100644 --- a/authbridge/cmd/abctl/tui/catalog_pane.go +++ b/authbridge/cmd/abctl/tui/catalog_pane.go @@ -51,9 +51,12 @@ func (m *model) rebuildCatalogTable() { } // selectedCatalogEntry returns the catalog entry under the cursor as -// a synthetic PipelinePlugin so showPluginDetail can render it. Direction -// is left blank and Position is 0 — showPluginDetail elides those fields -// when empty so the detail view degrades gracefully for catalog entries. +// a synthetic PipelinePlugin so showPluginDetail can render it. The +// singular Direction is left blank and Position is 0 — showPluginDetail +// elides those fields when empty so the detail view degrades gracefully +// for catalog entries. Directions (the type-level supported set) IS +// carried through: it is exactly the metadata a catalog browser wants, +// and it is meaningful without a configured instance. func (m *model) selectedCatalogEntry() *apiclient.PipelinePlugin { if m.catalog == nil { return nil @@ -68,6 +71,7 @@ func (m *model) selectedCatalogEntry() *apiclient.PipelinePlugin { if e.Name == name { p := apiclient.PipelinePlugin{ Name: e.Name, + Directions: e.Directions, ReadsBody: e.ReadsBody, Requires: e.Requires, RequiresAny: e.RequiresAny, diff --git a/authbridge/cmd/abctl/tui/edit_overlay.go b/authbridge/cmd/abctl/tui/edit_overlay.go index 2995a8020..a6adb601c 100644 --- a/authbridge/cmd/abctl/tui/edit_overlay.go +++ b/authbridge/cmd/abctl/tui/edit_overlay.go @@ -87,20 +87,34 @@ func renderEditOverlay(s editState, width, height int) string { // Validation banner: render BEFORE the diff so operators see // dependency issues at first glance. Non-blocking — apply still // works. - if len(s.validationErrs) > 0 { + // + // Errors and warnings get separate banners because they make + // different promises: an error means the framework's reload WILL + // reject the pipeline, while a warning (a plugin in a chain it + // doesn't declare) reloads fine and merely looks wrong. Folding + // the two together would make the "will reject" line false. + vErrs, vWarns := splitBySeverity(s.validationErrs) + if len(vErrs) > 0 { b.WriteString(styleError.Render(fmt.Sprintf( "⚠ %d validation issue%s — framework reload will reject:", - len(s.validationErrs), plural(len(s.validationErrs))))) + len(vErrs), plural(len(vErrs))))) b.WriteString("\n") - for _, ve := range s.validationErrs { - b.WriteString(fmt.Sprintf(" • [%s] %s pos %d: %s\n", - ve.Direction, ve.PluginName, ve.Position, ve.Message)) - } + writeValidationLines(&b, vErrs) + b.WriteString("\n") + } + if len(vWarns) > 0 { + b.WriteString(styleWarn.Render(fmt.Sprintf( + "%d advisory — reload will accept, but check:", + len(vWarns)))) + b.WriteString("\n") + writeValidationLines(&b, vWarns) b.WriteString("\n") } b.WriteString(s.diff) b.WriteString("\n") - if len(s.validationErrs) > 0 { + // "anyway" is warranted only when the framework will actually + // reject; an advisory doesn't change what apply does. + if len(vErrs) > 0 { b.WriteString(styleHint.Render("apply anyway? (y/N)")) } else { b.WriteString(styleHint.Render("apply this change? (y/N)")) @@ -128,3 +142,26 @@ func renderEditOverlay(s editState, width, height int) string { } return box.Render(b.String()) } + +// splitBySeverity partitions validation results into hard errors (the +// framework will reject) and advisories (it will accept). Order within +// each group is preserved so the chain/position sequence still reads +// top-to-bottom. +func splitBySeverity(in []edit.ValidationError) (errs, warns []edit.ValidationError) { + for _, ve := range in { + if ve.Severity == edit.SeverityWarning { + warns = append(warns, ve) + } else { + errs = append(errs, ve) + } + } + return errs, warns +} + +// writeValidationLines renders one bullet per validation result. +func writeValidationLines(b *strings.Builder, in []edit.ValidationError) { + for _, ve := range in { + b.WriteString(fmt.Sprintf(" • [%s] %s pos %d: %s\n", + ve.Direction, ve.PluginName, ve.Position, ve.Message)) + } +} diff --git a/authbridge/cmd/authbridge-cpex/main.go b/authbridge/cmd/authbridge-cpex/main.go index 050889df8..0d3a89e68 100644 --- a/authbridge/cmd/authbridge-cpex/main.go +++ b/authbridge/cmd/authbridge-cpex/main.go @@ -108,6 +108,7 @@ func main() { return nil, nil, nil, err } config.WarnEmptyPipelines(c, slog.Default()) + plugins.WarnPluginDirections(c, slog.Default()) in, err := plugins.BuildWithSPIFFE(c.Pipeline.Inbound.Plugins, provider) if err != nil { return nil, nil, nil, fmt.Errorf("inbound: %w", err) diff --git a/authbridge/cmd/authbridge-envoy/main.go b/authbridge/cmd/authbridge-envoy/main.go index 12fb43afe..57d61d3d0 100644 --- a/authbridge/cmd/authbridge-envoy/main.go +++ b/authbridge/cmd/authbridge-envoy/main.go @@ -129,6 +129,7 @@ func main() { return nil, nil, nil, err } config.WarnEmptyPipelines(c, slog.Default()) + plugins.WarnPluginDirections(c, slog.Default()) in, err := plugins.BuildWithSPIFFE(c.Pipeline.Inbound.Plugins, provider) if err != nil { return nil, nil, nil, fmt.Errorf("inbound: %w", err) diff --git a/authbridge/cmd/authbridge-proxy/catalog_test.go b/authbridge/cmd/authbridge-proxy/catalog_test.go new file mode 100644 index 000000000..a92882b6d --- /dev/null +++ b/authbridge/cmd/authbridge-proxy/catalog_test.go @@ -0,0 +1,95 @@ +package main + +import ( + "testing" + + "github.com/rossoctl/cortex/authbridge/authlib/sessionapi" +) + +// This binary links every default plugin, so it is the only place the +// FULL shipped catalog can be asserted. authlib's own tests see just the +// handful of plugins their side-effect imports pull in. +// +// Guards the two things a config generator reads off /v1/plugins: +// which chain each plugin belongs in, and its config field metadata. +func TestShippedCatalogPublishesDirections(t *testing.T) { + cat := sessionapi.PluginsCatalog() + if len(cat) == 0 { + t.Fatal("catalog is empty; plugin registration is broken") + } + for _, e := range cat { + if len(e.Directions) == 0 { + t.Errorf("plugin %q publishes no directions; a config generator "+ + "cannot place it in a chain", e.Name) + continue + } + for _, d := range e.Directions { + if d != "inbound" && d != "outbound" { + t.Errorf("plugin %q publishes unknown direction %q", e.Name, d) + } + } + // The singular Direction is positional and must stay empty here: + // the catalog describes types, not configured instances. + if e.Direction != "" { + t.Errorf("plugin %q: catalog should not set the positional Direction, got %q", + e.Name, e.Direction) + } + } +} + +// Field metadata for the plugins whose ConfigSchema this change added. +// Asserted here because litellm-budget-track is not linked into +// authlib's own test binaries. +// +// session-budget is deliberately absent: it is opt-IN +// (-tags include_plugin_sessionbudget, because it pulls in go-redis), so +// it is not part of the default build's catalog. Its schema is covered +// by authlib/plugins' TestConfigSchemaShapes when linked. +func TestShippedCatalogPublishesFieldSchemas(t *testing.T) { + want := map[string]int{ + "mcp-parser": 1, + "opa": 6, + "litellm-budget-track": 6, + } + got := map[string]int{} + for _, e := range sessionapi.PluginsCatalog() { + if _, tracked := want[e.Name]; tracked { + got[e.Name] = len(e.Fields) + } + } + for name, n := range want { + have, present := got[name] + if !present { + t.Errorf("%s is not in the shipped catalog", name) + continue + } + if have != n { + t.Errorf("%s publishes %d schema fields, want %d", name, have, n) + } + } +} + +// litellm-budget-track is the first plugin to expose float config, which +// is what motivated the "number" schema type. Without it these six +// fields would publish as "unknown" and render untyped in templates. +func TestShippedCatalogFloatFieldsAreNumbers(t *testing.T) { + for _, e := range sessionapi.PluginsCatalog() { + if e.Name != "litellm-budget-track" { + continue + } + for _, f := range e.Fields { + switch f.Name { + case "spend_file": + if f.Type != "string" { + t.Errorf("%s: type = %q, want string", f.Name, f.Type) + } + default: + if f.Type != "number" { + t.Errorf("%s: type = %q, want number", f.Name, f.Type) + } + } + } + return + } + t.Skip("litellm-budget-track not linked") +} diff --git a/authbridge/cmd/authbridge-proxy/main.go b/authbridge/cmd/authbridge-proxy/main.go index a2e85b819..b7499196a 100644 --- a/authbridge/cmd/authbridge-proxy/main.go +++ b/authbridge/cmd/authbridge-proxy/main.go @@ -235,6 +235,7 @@ func main() { return nil, nil, nil, err } config.WarnEmptyPipelines(c, slog.Default()) + plugins.WarnPluginDirections(c, slog.Default()) in, err := plugins.BuildWithSPIFFE(c.Pipeline.Inbound.Plugins, provider) if err != nil { return nil, nil, nil, fmt.Errorf("inbound: %w", err) diff --git a/authbridge/docs/plugin-catalog.md b/authbridge/docs/plugin-catalog.md index 450bbad7d..71530e96c 100644 --- a/authbridge/docs/plugin-catalog.md +++ b/authbridge/docs/plugin-catalog.md @@ -15,7 +15,13 @@ signal, not a claim about test coverage or operational maturity. ## Plugins "Direction" is inbound (caller → this agent) or outbound (this agent → -callee); "both" means the plugin evaluates on both pipelines. "Default +callee); "both" means the plugin evaluates on both pipelines. This column +is **declared in code** as `Capabilities().Directions` (see +`authlib/pipeline/plugin.go`), published on `/v1/plugins` as +`directions`, and asserted by a test — so the table below has a source of +truth rather than drifting. Placement is advisory: a plugin configured +into another chain still runs, but logs a startup WARN and shows an +advisory in `abctl`'s pre-apply validator. "Default config?" marks whether the plugin is enabled in Rossoctl's default AuthBridge pipeline YAML, not whether it is compiled into the binary (see "Production ready?" above for that).