From a4337f30920ee645cb9790f99d0591e1b7eb9820 Mon Sep 17 00:00:00 2001 From: ameersohel45 Date: Mon, 7 Sep 2026 13:22:43 +0530 Subject: [PATCH 01/10] fix(schemav2validator): keep the JSON-LD keys a schema declares [#16] Extended validation removed @context and @type from every object before validating it. That suited a schema which closes itself with additionalProperties:false and never mentions either key, and it made a schema pack that DECLARES @type and lists it in required impossible to satisfy: the payload carried @type, the schema required it, and the validator had just taken it out, so a conforming payload was rejected for a missing field it had supplied. Decide per key, by asking the schema whether it declares that key -- as a property or in required, anywhere in its composition tree, since the OAN packs declare @type one level down in allOf. Both schema styles then validate without a config switch and without either having to know about the other. Keeping @type also means its const is now checked, so a resource whose @type is not the one the capability declares no longer passes silently. [#16] Please enter the commit message for your changes. Lines starting [#16] with '#' will be ignored, and an empty message aborts the commit. [#16] [#16] Date: Mon Sep 7 13:22:43 2026 +0530 [#16] [#16] interactive rebase in progress; onto 983affb [#16] Last command done (1 command done): [#16] reword e19177c fix(schemav2validator): keep the JSON-LD keys a schema declares [#16] [#16] Next commands to do (5 remaining commands): [#16] reword 247022c refactor(plugins): name the capability plugins after their capabilities [#16] [#16] reword ec87958 feat(config): validate resource attributes against their schema packs [#16] [#16] You are currently editing a commit while rebasing branch 'tmp/rebase-16' on '983affb'. [#16] [#16] Changes to be committed: [#16] modified: pkg/plugin/implementation/schemav2validator/extended_schema.go [#16] modified: pkg/plugin/implementation/schemav2validator/extended_schema_test.go [#16] --- .../schemav2validator/extended_schema.go | 73 ++++++- .../schemav2validator/extended_schema_test.go | 193 ++++++++++++++++++ 2 files changed, 259 insertions(+), 7 deletions(-) diff --git a/pkg/plugin/implementation/schemav2validator/extended_schema.go b/pkg/plugin/implementation/schemav2validator/extended_schema.go index a002b019..654ba2d7 100644 --- a/pkg/plugin/implementation/schemav2validator/extended_schema.go +++ b/pkg/plugin/implementation/schemav2validator/extended_schema.go @@ -9,6 +9,7 @@ import ( "net/url" "os" "path/filepath" + "slices" "strings" "sync" "time" @@ -498,6 +499,69 @@ func isAllowedDomain(u *url.URL, allowedDomains []string) bool { return false } +// jsonLDKeys are the JSON-LD control keys that travel inside a domain object +// rather than beside it. +var jsonLDKeys = []string{"@context", "@type"} + +// stripUnaccountedJSONLDKeys removes those JSON-LD keys the target schema does +// not declare, and keeps the ones it does. +// +// The two schema styles in use need opposite treatment, and removing both keys +// unconditionally only served the first: +// +// - a schema that closes itself with additionalProperties:false and never +// mentions @type rejects the payload if @type is left in; +// - a schema pack that declares @type and lists it in required rejects the +// payload if @type is taken out. +// +// Asking the schema, per key, satisfies both without a config switch and +// without either style having to know about the other. +func stripUnaccountedJSONLDKeys(schema *openapi3.SchemaRef, data map[string]interface{}) map[string]interface{} { + domainData := make(map[string]interface{}, len(data)) + for k, v := range data { + if slices.Contains(jsonLDKeys, k) && !schemaDeclaresProperty(schema, k, map[*openapi3.Schema]bool{}) { + continue + } + domainData[k] = v + } + return domainData +} + +// schemaDeclaresProperty reports whether name is declared as a property, or +// listed as required, anywhere in a schema's composition tree. +// +// allOf, anyOf, oneOf and the then/else branches can each introduce a property, +// so all of them are walked -- the OAN packs declare @type one level down, in +// allOf. "not" is skipped because naming a property there forbids it rather +// than permitting it, and "if" is skipped because it only selects a branch. +// seen guards against schemas that reference themselves. +func schemaDeclaresProperty(ref *openapi3.SchemaRef, name string, seen map[*openapi3.Schema]bool) bool { + if ref == nil || ref.Value == nil || seen[ref.Value] { + return false + } + seen[ref.Value] = true + + if _, ok := ref.Value.Properties[name]; ok { + return true + } + if slices.Contains(ref.Value.Required, name) { + return true + } + for _, group := range []openapi3.SchemaRefs{ref.Value.AllOf, ref.Value.AnyOf, ref.Value.OneOf} { + for _, sub := range group { + if schemaDeclaresProperty(sub, name, seen) { + return true + } + } + } + for _, sub := range []*openapi3.SchemaRef{ref.Value.Then, ref.Value.Else} { + if schemaDeclaresProperty(sub, name, seen) { + return true + } + } + return false +} + // validateReferencedObject validates a single object with @context. func (c *schemaCache) validateReferencedObject( ctx context.Context, @@ -552,13 +616,8 @@ func (c *schemaCache) validateReferencedObject( return model.NewCodedErrorWithCause("SCH_INVALID_ENTITY_TYPE", err.Error(), obj.Path, err) } - // Strip JSON-LD metadata before validation - domainData := make(map[string]interface{}, len(obj.Data)-2) - for k, v := range obj.Data { - if k != "@context" && k != "@type" { - domainData[k] = v - } - } + // Strip only the JSON-LD keys this schema does not account for itself. + domainData := stripUnaccountedJSONLDKeys(schema, obj.Data) // Validate domain-specific data against schema opts := []openapi3.SchemaValidationOption{ diff --git a/pkg/plugin/implementation/schemav2validator/extended_schema_test.go b/pkg/plugin/implementation/schemav2validator/extended_schema_test.go index 6adc99be..e38325b1 100644 --- a/pkg/plugin/implementation/schemav2validator/extended_schema_test.go +++ b/pkg/plugin/implementation/schemav2validator/extended_schema_test.go @@ -9,6 +9,7 @@ import ( "os" "path/filepath" "reflect" + "sort" "sync/atomic" "testing" "time" @@ -1377,3 +1378,195 @@ func TestLoadSchemaFromPath_TTLExpiry_FetchesFresh(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "Schema v2", doc2.Info.Title, "expected v2 after TTL expiry — global URIMapCache not bypassed") } + +// packStyleSchema mirrors how the OAN schema packs are shaped: the capability +// declares @type one level down in allOf and lists it as required, and nothing +// closes the object with additionalProperties:false. +const packStyleSchema = `openapi: 3.1.0 +info: + title: Pack Style + version: 1.0.0 +components: + schemas: + WeatherObservation: + type: object + x-jsonld: + "@context": https://schemas.example.org/schema/WeatherObservation/v0.1/context.jsonld + "@type": openagrinet:WeatherObservation + allOf: + - type: object + required: + - informationMode + properties: + informationMode: + type: string + enum: [OnDemand, Direct] + - type: object + required: + - "@type" + properties: + "@type": + type: string + const: openagrinet:WeatherObservation` + +func writeTempSchema(t *testing.T, content string) string { + t.Helper() + f, err := os.CreateTemp("", "test-schema-*.yaml") + assert.NoError(t, err) + t.Cleanup(func() { os.Remove(f.Name()) }) + _, err = f.Write([]byte(content)) + assert.NoError(t, err) + assert.NoError(t, f.Close()) + return f.Name() +} + +// A pack that requires @type must receive it. This is the case that could not +// validate while both JSON-LD keys were removed unconditionally: the payload +// carries @type, the schema requires it, and stripping it produced a spurious +// "@type is required". +func TestValidateReferencedObject_PackStyleKeepsAtType(t *testing.T) { + cache := newSchemaCache(10) + path := writeTempSchema(t, packStyleSchema) + + obj := referencedObject{ + Path: "message.catalogs[0].resources[0].resourceAttributes", + Context: path, + Type: "openagrinet:WeatherObservation", + Data: map[string]interface{}{ + "@context": "https://schemas.example.org/schema/WeatherObservation/v0.1/context.jsonld", + "@type": "openagrinet:WeatherObservation", + "informationMode": "OnDemand", + }, + } + + err := cache.validateReferencedObject(context.Background(), obj, 1*time.Hour, 30*time.Second, nil, false) + assert.NoError(t, err) +} + +// The same pack must still reject a payload whose @type is not the one the +// capability declares -- keeping the key means its const is now checked, which +// stripping it silently skipped. +func TestValidateReferencedObject_PackStyleWrongAtTypeRejected(t *testing.T) { + cache := newSchemaCache(10) + path := writeTempSchema(t, packStyleSchema) + + obj := referencedObject{ + Path: "message.catalogs[0].resources[0].resourceAttributes", + Context: path, + Type: "openagrinet:WeatherObservation", + Data: map[string]interface{}{ + "@type": "openagrinet:MandiPrice", + "informationMode": "OnDemand", + }, + } + + err := cache.validateReferencedObject(context.Background(), obj, 1*time.Hour, 30*time.Second, nil, false) + assert.Error(t, err) +} + +func TestStripUnaccountedJSONLDKeys(t *testing.T) { + declaresType := &openapi3.SchemaRef{Value: &openapi3.Schema{ + AllOf: openapi3.SchemaRefs{ + {Value: &openapi3.Schema{ + Required: []string{"@type"}, + Properties: openapi3.Schemas{"@type": {Value: &openapi3.Schema{}}}, + }}, + }, + }} + declaresNeither := &openapi3.SchemaRef{Value: &openapi3.Schema{ + Properties: openapi3.Schemas{"field1": {Value: &openapi3.Schema{}}}, + }} + declaresBoth := &openapi3.SchemaRef{Value: &openapi3.Schema{ + Properties: openapi3.Schemas{ + "@context": {Value: &openapi3.Schema{}}, + "@type": {Value: &openapi3.Schema{}}, + }, + }} + + data := map[string]interface{}{ + "@context": "https://example.com/context.jsonld", + "@type": "openagrinet:WeatherObservation", + "field1": "value1", + } + + tests := []struct { + name string + schema *openapi3.SchemaRef + want []string + }{ + {"pack declares @type, so only @context goes", declaresType, []string{"@type", "field1"}}, + {"schema declares neither, so both go", declaresNeither, []string{"field1"}}, + {"schema declares both, so neither goes", declaresBoth, []string{"@context", "@type", "field1"}}, + {"nil schema is treated as declaring nothing", nil, []string{"field1"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := stripUnaccountedJSONLDKeys(tt.schema, data) + keys := make([]string, 0, len(got)) + for k := range got { + keys = append(keys, k) + } + sort.Strings(keys) + assert.Equal(t, tt.want, keys) + // the input must not be mutated -- obj.Data is shared with the caller + assert.Len(t, data, 3) + }) + } +} + +func TestSchemaDeclaresProperty(t *testing.T) { + leaf := func(required ...string) *openapi3.SchemaRef { + return &openapi3.SchemaRef{Value: &openapi3.Schema{Required: required}} + } + + cyclic := &openapi3.SchemaRef{Value: &openapi3.Schema{}} + cyclic.Value.AllOf = openapi3.SchemaRefs{cyclic} + + tests := []struct { + name string + schema *openapi3.SchemaRef + want bool + }{ + {"nil ref", nil, false}, + {"nil value", &openapi3.SchemaRef{}, false}, + {"declared directly as a property", &openapi3.SchemaRef{Value: &openapi3.Schema{ + Properties: openapi3.Schemas{"@type": {Value: &openapi3.Schema{}}}, + }}, true}, + {"required directly", leaf("@type"), true}, + {"required inside allOf", &openapi3.SchemaRef{Value: &openapi3.Schema{ + AllOf: openapi3.SchemaRefs{leaf("other"), leaf("@type")}, + }}, true}, + {"required inside anyOf", &openapi3.SchemaRef{Value: &openapi3.Schema{ + AnyOf: openapi3.SchemaRefs{leaf("@type")}, + }}, true}, + {"required inside oneOf", &openapi3.SchemaRef{Value: &openapi3.Schema{ + OneOf: openapi3.SchemaRefs{leaf("@type")}, + }}, true}, + {"required inside then", &openapi3.SchemaRef{Value: &openapi3.Schema{ + Then: leaf("@type"), + }}, true}, + {"required inside else", &openapi3.SchemaRef{Value: &openapi3.Schema{ + Else: leaf("@type"), + }}, true}, + // naming a property under "not" forbids it, so it must not count as declared + {"named under not does not count", &openapi3.SchemaRef{Value: &openapi3.Schema{ + Not: leaf("@type"), + }}, false}, + // "if" only selects a branch; it does not permit the property + {"named under if does not count", &openapi3.SchemaRef{Value: &openapi3.Schema{ + If: leaf("@type"), + }}, false}, + {"absent everywhere", &openapi3.SchemaRef{Value: &openapi3.Schema{ + AllOf: openapi3.SchemaRefs{leaf("informationMode")}, + }}, false}, + {"self-referencing schema terminates", cyclic, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := schemaDeclaresProperty(tt.schema, "@type", map[*openapi3.Schema]bool{}) + assert.Equal(t, tt.want, got) + }) + } +} From e524cf72194354bdf31a73cbd3e7d8a7f8465701 Mon Sep 17 00:00:00 2001 From: ameersohel45 Date: Mon, 7 Sep 2026 13:23:03 +0530 Subject: [PATCH 02/10] refactor(plugins): name the capability plugins after their capabilities [#16] weather and mandi named a domain; the payloads they serve name a capability. Renaming the packages to WeatherObservation and MandiPrice puts the two in the same vocabulary, so a binding key, a schema pack and the plugin that answers for it all read the same. The .so basename is the plugin id the adapter config refers to, so the directory rename carries the ids with it -- providerSteps and steps in config/oan-provider-adapter.yaml move together with the packages, and build-plugins.sh with them. No behaviour changes: the whole suite passes, and both plugins still build as loadable shared objects. Both package docs claimed one package per schema pack FAMILY, which the new names contradict. They now say what is true -- one package per capability, named for the capability, with the binding keys it answers to still configuration. [#16] Please enter the commit message for your changes. Lines starting [#16] with '#' will be ignored, and an empty message aborts the commit. [#16] [#16] Date: Mon Sep 7 13:23:03 2026 +0530 [#16] [#16] interactive rebase in progress; onto 983affb [#16] Last commands done (2 commands done): [#16] reword e19177c fix(schemav2validator): keep the JSON-LD keys a schema declares [#16] [#16] reword 247022c refactor(plugins): name the capability plugins after their capabilities [#16] [#16] Next commands to do (4 remaining commands): [#16] reword ec87958 feat(config): validate resource attributes against their schema packs [#16] [#16] reword bc8979b refactor(config): resolve capability schemas from the payload's @context [#16] [#16] You are currently editing a commit while rebasing branch 'tmp/rebase-16' on '983affb'. [#16] [#16] Changes to be committed: [#16] modified: config/oan-provider-adapter.yaml [#16] modified: install/build-plugins.sh [#16] renamed: pkg/plugin/implementation/mandi/mandi.go -> pkg/plugin/implementation/MandiPrice/MandiPrice.go [#16] renamed: pkg/plugin/implementation/mandi/cmd/plugin.go -> pkg/plugin/implementation/MandiPrice/cmd/plugin.go [#16] renamed: pkg/plugin/implementation/mandi/mappings_test.go -> pkg/plugin/implementation/MandiPrice/mappings_test.go [#16] renamed: pkg/plugin/implementation/mandi/prerequisites.go -> pkg/plugin/implementation/MandiPrice/prerequisites.go [#16] renamed: pkg/plugin/implementation/weather/weather.go -> pkg/plugin/implementation/WeatherObservation/WeatherObservation.go [#16] renamed: pkg/plugin/implementation/weather/cmd/plugin.go -> pkg/plugin/implementation/WeatherObservation/cmd/plugin.go [#16] renamed: pkg/plugin/implementation/weather/cmd/plugin_test.go -> pkg/plugin/implementation/WeatherObservation/cmd/plugin_test.go [#16] renamed: pkg/plugin/implementation/weather/mappings_test.go -> pkg/plugin/implementation/WeatherObservation/mappings_test.go [#16] renamed: pkg/plugin/implementation/weather/prerequisites.go -> pkg/plugin/implementation/WeatherObservation/prerequisites.go [#16] --- config/oan-provider-adapter.yaml | 12 ++++++------ install/build-plugins.sh | 4 ++-- .../mandi.go => MandiPrice/MandiPrice.go} | 10 +++++----- .../{mandi => MandiPrice}/cmd/plugin.go | 10 +++++----- .../{mandi => MandiPrice}/cmd/plugin_test.go | 18 +++++++++--------- .../{mandi => MandiPrice}/mappings_test.go | 16 ++++++++-------- .../{mandi => MandiPrice}/prerequisites.go | 2 +- .../WeatherObservation.go} | 13 ++++++++----- .../cmd/plugin.go | 10 +++++----- .../cmd/plugin_test.go | 12 ++++++------ .../mappings_test.go | 8 ++++---- .../prerequisites.go | 2 +- 12 files changed, 60 insertions(+), 57 deletions(-) rename pkg/plugin/implementation/{mandi/mandi.go => MandiPrice/MandiPrice.go} (81%) rename pkg/plugin/implementation/{mandi => MandiPrice}/cmd/plugin.go (91%) rename pkg/plugin/implementation/{mandi => MandiPrice}/cmd/plugin_test.go (95%) rename pkg/plugin/implementation/{mandi => MandiPrice}/mappings_test.go (98%) rename pkg/plugin/implementation/{mandi => MandiPrice}/prerequisites.go (98%) rename pkg/plugin/implementation/{weather/weather.go => WeatherObservation/WeatherObservation.go} (68%) rename pkg/plugin/implementation/{weather => WeatherObservation}/cmd/plugin.go (91%) rename pkg/plugin/implementation/{weather => WeatherObservation}/cmd/plugin_test.go (94%) rename pkg/plugin/implementation/{weather => WeatherObservation}/mappings_test.go (98%) rename pkg/plugin/implementation/{weather => WeatherObservation}/prerequisites.go (97%) diff --git a/config/oan-provider-adapter.yaml b/config/oan-provider-adapter.yaml index 34730d21..87bcd08c 100644 --- a/config/oan-provider-adapter.yaml +++ b/config/oan-provider-adapter.yaml @@ -228,7 +228,7 @@ modules: # rather than calling the provider unauthenticated. # ------------------------------------------------------------------ providerSteps: - - id: weather + - id: WeatherObservation config: # REQUIRED. Comma-separated, because a plugin config value is a # string and one provider may serve several capabilities. A @@ -278,9 +278,9 @@ modules: # A second capability in the same pipeline, from a different domain # package. Nothing about it is weather's business: a different # upstream, a different mapping, a different set of prerequisites -- - # and the same two registry rows. This entry, plus "mandi" in steps - # below, is the entire cost of adding it. - - id: mandi + # and the same two registry rows. This entry, plus "MandiPrice" + # in steps below, is the entire cost of adding it. + - id: MandiPrice config: bindingKeys: "agmarknet|openagrinet:MandiPrice" @@ -319,8 +319,8 @@ modules: steps: - validateSign # the sender's key, from the registry - validateSchema # the pinned Beckn v2 spec - - weather # openagrinet:WeatherObservation, or pass through - - mandi # openagrinet:MandiPrice, or pass through + - WeatherObservation # its binding key, or pass through + - MandiPrice # its binding key, or pass through - signAck # signs whatever the step answered with # ---------------------------------------------------------------------------- diff --git a/install/build-plugins.sh b/install/build-plugins.sh index 8ac0d713..141cb3b0 100755 --- a/install/build-plugins.sh +++ b/install/build-plugins.sh @@ -32,8 +32,8 @@ plugins=( "dediregistry" "oanregistry" "jsonmapper" - "weather" - "mandi" + "WeatherObservation" + "MandiPrice" "manifestloader" "reqpreprocessor" "otelsetup" diff --git a/pkg/plugin/implementation/mandi/mandi.go b/pkg/plugin/implementation/MandiPrice/MandiPrice.go similarity index 81% rename from pkg/plugin/implementation/mandi/mandi.go rename to pkg/plugin/implementation/MandiPrice/MandiPrice.go index 26e8b94b..38fff611 100644 --- a/pkg/plugin/implementation/mandi/mandi.go +++ b/pkg/plugin/implementation/MandiPrice/MandiPrice.go @@ -1,8 +1,8 @@ -// Package mandi serves the network's market price capabilities. +// Package MandiPrice serves the network's market price capabilities. // -// One package per schema pack family, so which plugin owns a capability is -// readable from its binding key: openagrinet:MandiPrice is mandi's, -// openagrinet:WeatherObservation is weather's. +// One package per capability, named for the capability it serves, so which +// plugin owns a payload is readable from its binding key without a lookup: +// openagrinet:MandiPrice is this one's, openagrinet:WeatherObservation is not. // // Almost nothing lives here, and that is the point. Recognising a capability, // resolving the call plan, authenticating, calling with the registry's budget @@ -15,7 +15,7 @@ // select takes governed codes for state, district, market and commodity plus a // date range, all of which a MandiPrice payload carries. So the package is a // name and nothing else: see prerequisites.go for why that is worth stating. -package mandi +package MandiPrice import ( "context" diff --git a/pkg/plugin/implementation/mandi/cmd/plugin.go b/pkg/plugin/implementation/MandiPrice/cmd/plugin.go similarity index 91% rename from pkg/plugin/implementation/mandi/cmd/plugin.go rename to pkg/plugin/implementation/MandiPrice/cmd/plugin.go index 5b4fbb70..8dcce23f 100644 --- a/pkg/plugin/implementation/mandi/cmd/plugin.go +++ b/pkg/plugin/implementation/MandiPrice/cmd/plugin.go @@ -13,20 +13,20 @@ import ( "github.com/beckn-one/beckn-onix/pkg/log" "github.com/beckn-one/beckn-onix/pkg/plugin/definition" - "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/mandi" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/MandiPrice" ) // mandiProvider implements definition.ProviderStepProvider. type mandiProvider struct{} // newStepFunc creates a new step. Indirected for tests. -var newStepFunc = mandi.New +var newStepFunc = MandiPrice.New // parseConfig turns the plugin config map into a typed Config. Anything absent -// is left zero: mandi.New applies the defaults and validates the auth scheme, +// is left zero: MandiPrice.New applies the defaults and validates the auth scheme, // so those rules live in one place. -func (p mandiProvider) parseConfig(config map[string]string) (*mandi.Config, error) { - cfg := &mandi.Config{ +func (p mandiProvider) parseConfig(config map[string]string) (*MandiPrice.Config, error) { + cfg := &MandiPrice.Config{ BindingKeys: splitList(config["bindingKeys"]), // Absent means the Beckn v2 convention. See upstream.Config for why // this is a default rather than something to set. diff --git a/pkg/plugin/implementation/mandi/cmd/plugin_test.go b/pkg/plugin/implementation/MandiPrice/cmd/plugin_test.go similarity index 95% rename from pkg/plugin/implementation/mandi/cmd/plugin_test.go rename to pkg/plugin/implementation/MandiPrice/cmd/plugin_test.go index 262f73a6..0a3c1a50 100644 --- a/pkg/plugin/implementation/mandi/cmd/plugin_test.go +++ b/pkg/plugin/implementation/MandiPrice/cmd/plugin_test.go @@ -9,7 +9,7 @@ import ( "github.com/beckn-one/beckn-onix/pkg/model" "github.com/beckn-one/beckn-onix/pkg/plugin/definition" - "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/mandi" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/MandiPrice" ) type stubRegistry struct{} @@ -32,15 +32,15 @@ func TestParseConfig(t *testing.T) { testCases := []struct { name string config map[string]string - expected *mandi.Config + expected *MandiPrice.Config expectedErr string }{ { - // Everything absent is left zero: mandi.New defaults it, so the + // Everything absent is left zero: MandiPrice.New defaults it, so the // rules are defined in exactly one place. name: "leaves everything unset for New to default", config: map[string]string{}, - expected: &mandi.Config{}, + expected: &MandiPrice.Config{}, }, { // Query auth is why this capability has its own entry rather than @@ -54,7 +54,7 @@ func TestParseConfig(t *testing.T) { "queryName": "api-key", "queryValueEnv": "MANDI_TOKEN", }, - expected: &mandi.Config{ + expected: &MandiPrice.Config{ BindingKeys: []string{"agmarknet|openagrinet:MandiPrice"}, AuthScheme: "query", QueryName: "api-key", @@ -74,7 +74,7 @@ func TestParseConfig(t *testing.T) { "queryValueEnv": "Q", "maxResponseBytes": "2048", }, - expected: &mandi.Config{ + expected: &MandiPrice.Config{ BindingKeys: []string{"other|capability"}, AuthScheme: "basic", UsernameEnv: "U", @@ -102,7 +102,7 @@ func TestParseConfig(t *testing.T) { // as "unset" rather than failing startup. name: "treats an empty response cap as unset", config: map[string]string{"maxResponseBytes": ""}, - expected: &mandi.Config{}, + expected: &MandiPrice.Config{}, }, } @@ -243,7 +243,7 @@ func TestNew(t *testing.T) { closed := false original := newStepFunc newStepFunc = func(context.Context, definition.ProviderRecordLookup, definition.Mapper, - *mandi.Config) (definition.Step, func() error, error) { + *MandiPrice.Config) (definition.Step, func() error, error) { return nil, func() error { closed = true; return nil }, nil } defer func() { newStepFunc = original }() @@ -268,7 +268,7 @@ func TestNew(t *testing.T) { original := newStepFunc wanted := errors.New("upstream refused the config") newStepFunc = func(context.Context, definition.ProviderRecordLookup, definition.Mapper, - *mandi.Config) (definition.Step, func() error, error) { + *MandiPrice.Config) (definition.Step, func() error, error) { return nil, nil, wanted } defer func() { newStepFunc = original }() diff --git a/pkg/plugin/implementation/mandi/mappings_test.go b/pkg/plugin/implementation/MandiPrice/mappings_test.go similarity index 98% rename from pkg/plugin/implementation/mandi/mappings_test.go rename to pkg/plugin/implementation/MandiPrice/mappings_test.go index 6be53f46..ef91a40c 100644 --- a/pkg/plugin/implementation/mandi/mappings_test.go +++ b/pkg/plugin/implementation/MandiPrice/mappings_test.go @@ -1,4 +1,4 @@ -package mandi_test +package MandiPrice_test // mappings_test.go runs the shipped mandi mapping through the real mapper and // the real provider step. It is the only test that proves the three pieces fit: @@ -22,8 +22,8 @@ import ( "testing" "github.com/beckn-one/beckn-onix/pkg/model" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/MandiPrice" "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/jsonmapper" - "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/mandi" ) // mappingsDir is where the shipped mappings live, relative to this package. @@ -191,8 +191,8 @@ func runShippedWith(t *testing.T, request, providerBody string) (url.Values, map }, }} - step, closeStep, err := mandi.New(context.Background(), registry, mapper, - &mandi.Config{BindingKeys: []string{shippedBindingKey}}) + step, closeStep, err := MandiPrice.New(context.Background(), registry, mapper, + &MandiPrice.Config{BindingKeys: []string{shippedBindingKey}}) if err != nil { t.Fatalf("failed to build the step: %v", err) } @@ -431,8 +431,8 @@ func TestShippedMappingRefusesWhatItCannotServe(t *testing.T) { Mappings: mappings.URL + "/" + shippedMapping, TimeoutMs: 30000}, }, }} - step, closeStep, err := mandi.New(context.Background(), registry, mapper, - &mandi.Config{BindingKeys: []string{shippedBindingKey}}) + step, closeStep, err := MandiPrice.New(context.Background(), registry, mapper, + &MandiPrice.Config{BindingKeys: []string{shippedBindingKey}}) if err != nil { t.Fatalf("failed to build the step: %v", err) } @@ -740,8 +740,8 @@ func TestShippedMappingRefusesPayloadsItCannotAnswer(t *testing.T) { Mappings: mappings.URL + "/" + shippedMapping, TimeoutMs: 30000}, }, }} - step, closeStep, err := mandi.New(context.Background(), registry, mapper, - &mandi.Config{BindingKeys: []string{shippedBindingKey}}) + step, closeStep, err := MandiPrice.New(context.Background(), registry, mapper, + &MandiPrice.Config{BindingKeys: []string{shippedBindingKey}}) if err != nil { t.Fatalf("failed to build the step: %v", err) } diff --git a/pkg/plugin/implementation/mandi/prerequisites.go b/pkg/plugin/implementation/MandiPrice/prerequisites.go similarity index 98% rename from pkg/plugin/implementation/mandi/prerequisites.go rename to pkg/plugin/implementation/MandiPrice/prerequisites.go index 2027248b..d583092c 100644 --- a/pkg/plugin/implementation/mandi/prerequisites.go +++ b/pkg/plugin/implementation/MandiPrice/prerequisites.go @@ -1,4 +1,4 @@ -package mandi +package MandiPrice import "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/upstream" diff --git a/pkg/plugin/implementation/weather/weather.go b/pkg/plugin/implementation/WeatherObservation/WeatherObservation.go similarity index 68% rename from pkg/plugin/implementation/weather/weather.go rename to pkg/plugin/implementation/WeatherObservation/WeatherObservation.go index b899e45b..65820e84 100644 --- a/pkg/plugin/implementation/weather/weather.go +++ b/pkg/plugin/implementation/WeatherObservation/WeatherObservation.go @@ -1,15 +1,18 @@ -// Package weather serves the network's weather capabilities. +// Package WeatherObservation serves the network's weather capabilities. // -// One package per schema pack family, so which plugin owns a capability is -// readable from its binding key: openagrinet:WeatherObservation and -// openagrinet:WeatherAdvisory are weather's, openagrinet:MandiPrice is not. +// One package per capability, named for the capability it serves, so which +// plugin owns a payload is readable from its binding key without a lookup: +// openagrinet:WeatherObservation is this one's, openagrinet:MandiPrice is not. +// Which keys it answers to is still configuration -- a deployment can point it +// at a related pack such as openagrinet:WeatherAdvisory -- but the name says +// what it was built against. // // Almost nothing lives here. Recognising a capability, resolving the call plan, // authenticating, calling with the registry's budget and translating in both // directions are all internal/upstream's, because none of them differ by domain. // What this package owns is its name, and prerequisites -- the work a mapping // cannot express, which is domain knowledge by definition. -package weather +package WeatherObservation import ( "context" diff --git a/pkg/plugin/implementation/weather/cmd/plugin.go b/pkg/plugin/implementation/WeatherObservation/cmd/plugin.go similarity index 91% rename from pkg/plugin/implementation/weather/cmd/plugin.go rename to pkg/plugin/implementation/WeatherObservation/cmd/plugin.go index e78c17ef..9a53bf97 100644 --- a/pkg/plugin/implementation/weather/cmd/plugin.go +++ b/pkg/plugin/implementation/WeatherObservation/cmd/plugin.go @@ -9,20 +9,20 @@ import ( "github.com/beckn-one/beckn-onix/pkg/log" "github.com/beckn-one/beckn-onix/pkg/plugin/definition" - "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/weather" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/WeatherObservation" ) // weatherProvider implements definition.ProviderStepProvider. type weatherProvider struct{} // newStepFunc creates a new step. Indirected for tests. -var newStepFunc = weather.New +var newStepFunc = WeatherObservation.New // parseConfig turns the plugin config map into a typed Config. Anything absent -// is left zero: weather.New applies the defaults and validates the auth +// is left zero: WeatherObservation.New applies the defaults and validates the auth // scheme, so those rules live in one place. -func (p weatherProvider) parseConfig(config map[string]string) (*weather.Config, error) { - cfg := &weather.Config{ +func (p weatherProvider) parseConfig(config map[string]string) (*WeatherObservation.Config, error) { + cfg := &WeatherObservation.Config{ BindingKeys: splitList(config["bindingKeys"]), // Absent means the Beckn v2 convention. See upstream.Config for why // this is a default rather than something to set. diff --git a/pkg/plugin/implementation/weather/cmd/plugin_test.go b/pkg/plugin/implementation/WeatherObservation/cmd/plugin_test.go similarity index 94% rename from pkg/plugin/implementation/weather/cmd/plugin_test.go rename to pkg/plugin/implementation/WeatherObservation/cmd/plugin_test.go index e04b9191..f95323da 100644 --- a/pkg/plugin/implementation/weather/cmd/plugin_test.go +++ b/pkg/plugin/implementation/WeatherObservation/cmd/plugin_test.go @@ -9,7 +9,7 @@ import ( "github.com/beckn-one/beckn-onix/pkg/model" "github.com/beckn-one/beckn-onix/pkg/plugin/definition" - "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/weather" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/WeatherObservation" ) type stubRegistry struct{} @@ -32,15 +32,15 @@ func TestParseConfig(t *testing.T) { testCases := []struct { name string config map[string]string - expected *weather.Config + expected *WeatherObservation.Config expectedErr string }{ { - // Everything absent is left zero: weather.New defaults it, so the + // Everything absent is left zero: WeatherObservation.New defaults it, so the // rules are defined in exactly one place. name: "leaves everything unset for New to default", config: map[string]string{}, - expected: &weather.Config{}, + expected: &WeatherObservation.Config{}, }, { name: "reads every supported setting", @@ -53,7 +53,7 @@ func TestParseConfig(t *testing.T) { "headerValueEnv": "V", "maxResponseBytes": "2048", }, - expected: &weather.Config{ + expected: &WeatherObservation.Config{ BindingKeys: []string{"other|capability"}, AuthScheme: "basic", UsernameEnv: "U", @@ -222,7 +222,7 @@ func TestNew(t *testing.T) { t.Cleanup(func() { newStepFunc = original }) wantErr := errors.New("boom") - newStepFunc = func(context.Context, definition.ProviderRecordLookup, definition.Mapper, *weather.Config) (definition.Step, func() error, error) { + newStepFunc = func(context.Context, definition.ProviderRecordLookup, definition.Mapper, *WeatherObservation.Config) (definition.Step, func() error, error) { return nil, nil, wantErr } diff --git a/pkg/plugin/implementation/weather/mappings_test.go b/pkg/plugin/implementation/WeatherObservation/mappings_test.go similarity index 98% rename from pkg/plugin/implementation/weather/mappings_test.go rename to pkg/plugin/implementation/WeatherObservation/mappings_test.go index 4c275765..b25c04d1 100644 --- a/pkg/plugin/implementation/weather/mappings_test.go +++ b/pkg/plugin/implementation/WeatherObservation/mappings_test.go @@ -1,4 +1,4 @@ -package weather_test +package WeatherObservation_test // mappings_test.go runs the shipped mapping files through the real mapper and // the real provider step. It is the only test that proves the three pieces fit: @@ -22,8 +22,8 @@ import ( "github.com/beckn-one/beckn-onix/pkg/model" "github.com/beckn-one/beckn-onix/pkg/plugin/definition" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/WeatherObservation" "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/jsonmapper" - "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/weather" ) // mappingsDir is where the shipped mappings live, relative to this package. @@ -142,8 +142,8 @@ func TestShippedMappingsServeARealSelect(t *testing.T) { }, }} - step, closeStep, err := weather.New(context.Background(), registry, mapper, - &weather.Config{BindingKeys: []string{shippedBindingKey}}) + step, closeStep, err := WeatherObservation.New(context.Background(), registry, mapper, + &WeatherObservation.Config{BindingKeys: []string{shippedBindingKey}}) if err != nil { t.Fatalf("failed to build the step: %v", err) } diff --git a/pkg/plugin/implementation/weather/prerequisites.go b/pkg/plugin/implementation/WeatherObservation/prerequisites.go similarity index 97% rename from pkg/plugin/implementation/weather/prerequisites.go rename to pkg/plugin/implementation/WeatherObservation/prerequisites.go index 0c7b947a..3492d577 100644 --- a/pkg/plugin/implementation/weather/prerequisites.go +++ b/pkg/plugin/implementation/WeatherObservation/prerequisites.go @@ -1,4 +1,4 @@ -package weather +package WeatherObservation import "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/upstream" From 586ed23cc1184e5832f7463514bab70ed63a92eb Mon Sep 17 00:00:00 2001 From: ameersohel45 Date: Mon, 7 Sep 2026 13:23:24 +0530 Subject: [PATCH 03/10] feat(config): validate resource attributes against their schema packs [#16] The base validator treats resourceAttributes as a free-form object -- the envelope is correct whatever a capability puts inside it. Extended validation checks the inside: it walks the payload for objects carrying @context and @type, resolves the schema @type names, and validates against it. Until the JSON-LD strip was fixed no pack could pass, so the keys were configured and the layer was off. Resolution is local, not a fetch per payload. Every schema under extendedSchema_localSchemaPath is loaded at startup and looked up by @type, so a select costs no network call and works with no egress. The schemas are published elsewhere and deliberately not copied in here; a copy would drift, and pinning a revision in adapter config would make a deployment decision on the deployment's behalf. The comment states the layout to mount, $ref targets included. A missing directory fails startup rather than degrading, which is the behaviour to want -- the alternative is accepting unvalidated payloads because a mount was forgotten. The allowlist is narrowed from raw.githubusercontent.com to the host the packs' own @context names, so a local miss fails loudly instead of quietly fetching from elsewhere. Recorded in the comment because a green result is otherwise misleading: the validator library parses if/then/else but never evaluates it, so a pack's conditional rules are not enforced. In the OAN packs that is everything predicated on informationMode. [#16] Please enter the commit message for your changes. Lines starting [#16] with '#' will be ignored, and an empty message aborts the commit. [#16] [#16] Date: Mon Sep 7 13:23:24 2026 +0530 [#16] [#16] interactive rebase in progress; onto 983affb [#16] Last commands done (3 commands done): [#16] reword 247022c refactor(plugins): name the capability plugins after their capabilities [#16] [#16] reword ec87958 feat(config): validate resource attributes against their schema packs [#16] [#16] Next commands to do (3 remaining commands): [#16] reword bc8979b refactor(config): resolve capability schemas from the payload's @context [#16] [#16] reword 6385051 refactor: name the registry and binding packages for what they are [#16] [#16] You are currently editing a commit while rebasing branch 'tmp/rebase-16' on '983affb'. [#16] [#16] Changes to be committed: [#16] modified: config/oan-provider-adapter.yaml [#16] --- config/oan-provider-adapter.yaml | 72 ++++++++++++++++++++++++++++---- 1 file changed, 64 insertions(+), 8 deletions(-) diff --git a/config/oan-provider-adapter.yaml b/config/oan-provider-adapter.yaml index 87bcd08c..d7e1812e 100644 --- a/config/oan-provider-adapter.yaml +++ b/config/oan-provider-adapter.yaml @@ -154,22 +154,78 @@ modules: signValidator: id: signvalidator - # Base Beckn v2 schema validation against the pinned LTS spec. The - # extended layer fetches each resource's own @context and validates - # against that -- a network call per payload and a second thing that - # can fail -- so it is off, and the extendedSchema_* keys below only - # take effect if it is switched on. + # ------------------------------------------------------------------ + # schemaValidator -- two layers, both on. + # + # BASE validates the envelope against the pinned Beckn v2 LTS spec. + # To it, resourceAttributes is a free-form object: the envelope is + # correct whatever a capability puts inside. + # + # EXTENDED validates that inside. It walks the payload for every + # object carrying both @context and @type, resolves the schema that + # @type names, and validates the object against it. That is what makes + # a wrong unit or a missing required attribute a rejected payload + # rather than a provider's problem to discover later. + # + # What extended validation DOES enforce: types, string formats + # (date-time, duration, uri), enum, const, required, minItems, + # additionalProperties, not, and allOf/anyOf/oneOf. + # + # What it does NOT: if/then/else. The validator library parses those + # keywords but never evaluates them, so a pack's conditional rules -- + # in the OAN packs, everything predicated on informationMode -- are + # not checked. Worth knowing before treating a pass here as full + # conformance to a pack. + # ------------------------------------------------------------------ schemaValidator: id: schemav2validator config: type: url location: "https://raw.githubusercontent.com/beckn/protocol-specifications-v2/refs/tags/core-v2.0.0-lts/api/v2.0.0/beckn.yaml" cacheTTL: "3600" - extendedSchema_enabled: "false" - extendedSchema_cacheTTL: "86400" + + extendedSchema_enabled: "true" + + # Where the capability schemas are read from. Set, so resolution + # is a memory lookup and no payload costs a network call. + # + # Every *.yaml under this directory is loaded at STARTUP and keyed + # by /attributes.yaml, with the version segment dropped. + # An object's @type is then looked up directly -- the part after + # the colon, so openagrinet:MandiPrice finds MandiPrice. $refs + # between schemas resolve out of the same memory. + # + # The layout to place there, $ref targets included: + # + # WeatherObservation/v0.1/attributes.yaml + # MandiPrice/v0.1/attributes.yaml + # AgricultureResource/v0.1/attributes.yaml # the packs' base + # Address/v2.0/attributes.yaml # schema.beckn.io + # Descriptor/v2.1/attributes.yaml # schema.beckn.io + # GeoJSONGeometry/v2.0/attributes.yaml # schema.beckn.io + # Location/v2.0/attributes.yaml # schema.beckn.io + # + # The schemas are NOT in this repository: they are published, and + # a second copy here would drift from them. The deployment places + # them at this path, which is also why no source URL appears in + # this file -- which revision to ship is a deployment decision, + # not one frozen into adapter config. + # + # A missing directory FAILS STARTUP rather than degrading, which + # is what you want: the alternative is an adapter that accepts + # unvalidated payloads because a mount was forgotten. An empty one + # only warns, and then every payload falls through to the network. + extendedSchema_localSchemaPath: "/app/config/schemas" + + # The network fallback, reached only on a local miss. Restricted + # to the host the packs' own @context names, so a miss fails + # loudly instead of quietly fetching a schema from somewhere else. + extendedSchema_allowedDomains: "schemas.openagrinet.global" + + # The three keys below apply to that fallback only. + extendedSchema_cacheTTL: "86400" # 24h extendedSchema_maxCacheSize: "100" extendedSchema_downloadTimeout: "30" - extendedSchema_allowedDomains: "raw.githubusercontent.com" # ------------------------------------------------------------------ # jsonmapper -- the JSONata mapper. From 4fdf394d4be5724bde6b8f91fbd8205e0e178b1d Mon Sep 17 00:00:00 2001 From: ameersohel45 Date: Mon, 7 Sep 2026 14:17:15 +0530 Subject: [PATCH 04/10] refactor(config): resolve capability schemas from the payload's @context [#16] Extended validation was pointed at a mounted directory of schema files, because the @context the payloads declared -- schemas.openagrinet.global -- does not resolve, and a failed fetch rejects the payload. That put the burden on every deployment to place the right files at the right path, and made the adapter refuse to start when one did not. The published packs do serve context.jsonld, so the fetch the validator already knows how to do works once @context names them: @context .../network-specs/schema-packs-v0.1/schema/MandiPrice/v0.1/context.jsonld fetched .../network-specs/schema-packs-v0.1/schema/MandiPrice/v0.1/attributes.yaml So localSchemaPath is gone, and with it the mount, the directory and the script that filled it. The revision a payload is judged against now comes from the payload, which is a better place for it than adapter config: nothing here can go stale against what the network publishes. The allowlist moves to raw.githubusercontent.com, the host that @context now resolves to. It is doing real work rather than naming a host that never answered -- an @context on any other host is refused before a fetch is attempted. Two costs, both stated in the config: this adapter now needs egress to that host, and the first payload after a restart pays for the fetch. Measured at about 2s, cached for 24h after that. Verified in oan-local: publish and select both pass, the first payload logs "fetching from network" and later ones "LRU cache hit", a foreign @context is refused with SCH_INVALID_JSONLD_CONTEXT, and the collection is 51 of 51. [#16] Please enter the commit message for your changes. Lines starting [#16] with '#' will be ignored, and an empty message aborts the commit. [#16] [#16] Date: Mon Sep 7 14:17:15 2026 +0530 [#16] [#16] interactive rebase in progress; onto 983affb [#16] Last commands done (4 commands done): [#16] reword ec87958 feat(config): validate resource attributes against their schema packs [#16] [#16] reword bc8979b refactor(config): resolve capability schemas from the payload's @context [#16] [#16] Next commands to do (2 remaining commands): [#16] reword 6385051 refactor: name the registry and binding packages for what they are [#16] [#16] reword 9ebb5c1 docs(config): placeholder the subscriber id, keep the old value in a comment [#16] [#16] You are currently editing a commit while rebasing branch 'tmp/rebase-16' on '983affb'. [#16] [#16] Changes to be committed: [#16] modified: config/oan-provider-adapter.yaml [#16] --- config/oan-provider-adapter.yaml | 53 ++++++++++++-------------------- 1 file changed, 20 insertions(+), 33 deletions(-) diff --git a/config/oan-provider-adapter.yaml b/config/oan-provider-adapter.yaml index d7e1812e..d3694a63 100644 --- a/config/oan-provider-adapter.yaml +++ b/config/oan-provider-adapter.yaml @@ -167,6 +167,10 @@ modules: # a wrong unit or a missing required attribute a rejected payload # rather than a provider's problem to discover later. # + # The schemas are not in this repository and are not mounted: they + # are fetched from the @context a payload declares, so the revision + # is the payload's choice and nothing here can go stale against it. + # # What extended validation DOES enforce: types, string formats # (date-time, duration, uri), enum, const, required, minItems, # additionalProperties, not, and allOf/anyOf/oneOf. @@ -186,43 +190,26 @@ modules: extendedSchema_enabled: "true" - # Where the capability schemas are read from. Set, so resolution - # is a memory lookup and no payload costs a network call. - # - # Every *.yaml under this directory is loaded at STARTUP and keyed - # by /attributes.yaml, with the version segment dropped. - # An object's @type is then looked up directly -- the part after - # the colon, so openagrinet:MandiPrice finds MandiPrice. $refs - # between schemas resolve out of the same memory. + # Resolution is a FETCH of the @context each resource declares, + # not a directory mounted here. The validator swaps context.jsonld + # for attributes.yaml to get the schema beside it: # - # The layout to place there, $ref targets included: + # @context .../schema/MandiPrice/v0.1/context.jsonld + # fetched .../schema/MandiPrice/v0.1/attributes.yaml # - # WeatherObservation/v0.1/attributes.yaml - # MandiPrice/v0.1/attributes.yaml - # AgricultureResource/v0.1/attributes.yaml # the packs' base - # Address/v2.0/attributes.yaml # schema.beckn.io - # Descriptor/v2.1/attributes.yaml # schema.beckn.io - # GeoJSONGeometry/v2.0/attributes.yaml # schema.beckn.io - # Location/v2.0/attributes.yaml # schema.beckn.io + # So a payload names the pack revision it wants to be judged + # against, and no copy of the schemas here can drift from the + # published ones. The packs' relative $refs (into + # AgricultureResource) resolve against that same base; their + # absolute ones resolve directly, against whichever host they name. # - # The schemas are NOT in this repository: they are published, and - # a second copy here would drift from them. The deployment places - # them at this path, which is also why no source URL appears in - # this file -- which revision to ship is a deployment decision, - # not one frozen into adapter config. - # - # A missing directory FAILS STARTUP rather than degrading, which - # is what you want: the alternative is an adapter that accepts - # unvalidated payloads because a mount was forgotten. An empty one - # only warns, and then every payload falls through to the network. - extendedSchema_localSchemaPath: "/app/config/schemas" - - # The network fallback, reached only on a local miss. Restricted - # to the host the packs' own @context names, so a miss fails - # loudly instead of quietly fetching a schema from somewhere else. - extendedSchema_allowedDomains: "schemas.openagrinet.global" + # Fetched once per @context and cached for the TTL below, so only + # the first payload after a restart pays for it. A fetch that + # FAILS rejects the payload -- it does not skip validation, which + # is the right way round, but it does mean this adapter needs + # egress to the host allowed below. + extendedSchema_allowedDomains: "raw.githubusercontent.com" - # The three keys below apply to that fallback only. extendedSchema_cacheTTL: "86400" # 24h extendedSchema_maxCacheSize: "100" extendedSchema_downloadTimeout: "30" From 4d18317353c428ac7c29729e070c082d6c9db6c8 Mon Sep 17 00:00:00 2001 From: ameersohel45 Date: Mon, 7 Sep 2026 17:15:52 +0530 Subject: [PATCH 05/10] refactor: name the registry and binding packages for what they are [#16] Three renames, and one rule applied across the tree. pkg/plugin/implementation/oanregistry -> sunbirdRegistry pkg/plugin/implementation/internal/oanbinding -> internal/capabilitybinding config/oan-provider-adapter.yaml -> config/provider-adapter.yaml capabilitybinding rather than keybinding, which was the suggestion: this package derives a {ParticipantID, CapabilityCode} pair and renders it as the participant|capability key the registry indexes on. "Capability binding" is already the term the code, the registry schema and the config all use, so the package now says it. "Key" would have been actively misleading -- this codebase uses key for signing keys, which is a different thing resolved by a different plugin. The organisation name is gone from every filename, identifier, comment, error string and trace span name. Two categories were deliberately left alone because they are DATA rather than naming: - openagrinet:WeatherObservation and its siblings are capability codes on the wire. Changing them would change the protocol. - participant ids and names inside captured registry fixtures (provider.oan.local, "OAN provider layer adapter") are verbatim responses. Editing them would make the fixture stop matching what a registry returns, which is the only reason the fixture is worth having. Two consequences worth knowing. The .so basename is the plugin id, so `id: oanregistry` becomes `id: sunbirdRegistry` and any deployment's config moves with the image. And pluginID is a telemetry attribute, so traces and metrics from this plugin now report sunbirdRegistry -- it follows the rename rather than reporting a name that no longer exists. The two cache key prefixes changed with it, oan_lookup_ and oan_provider_ to registry_lookup_ and registry_provider_. They are cache namespaces, not metric names, so the cost is one cold cache cycle. Full suite green at 63 packages, vet clean, and sunbirdRegistry.so builds. [#16] Please enter the commit message for your changes. Lines starting [#16] with '#' will be ignored, and an empty message aborts the commit. [#16] [#16] Date: Mon Sep 7 17:15:52 2026 +0530 [#16] [#16] interactive rebase in progress; onto 983affb [#16] Last commands done (5 commands done): [#16] reword bc8979b refactor(config): resolve capability schemas from the payload's @context [#16] [#16] reword 6385051 refactor: name the registry and binding packages for what they are [#16] [#16] Next command to do (1 remaining command): [#16] reword 9ebb5c1 docs(config): placeholder the subscriber id, keep the old value in a comment [#16] [#16] You are currently editing a commit while rebasing branch 'tmp/rebase-16' on '983affb'. [#16] [#16] Changes to be committed: [#16] modified: config/mappings/agmarknet/mandi-price.select.yaml [#16] renamed: config/oan-provider-adapter.yaml -> config/provider-adapter.yaml [#16] modified: install/build-plugins.sh [#16] modified: pkg/plugin/definition/mapper.go [#16] modified: pkg/plugin/implementation/WeatherObservation/mappings_test.go [#16] renamed: pkg/plugin/implementation/internal/oanbinding/oanbinding.go -> pkg/plugin/implementation/internal/capabilitybinding/capabilitybinding.go [#16] renamed: pkg/plugin/implementation/internal/oanbinding/oanbinding_test.go -> pkg/plugin/implementation/internal/capabilitybinding/capabilitybinding_test.go [#16] renamed: pkg/plugin/implementation/internal/oanbinding/paths.go -> pkg/plugin/implementation/internal/capabilitybinding/paths.go [#16] modified: pkg/plugin/implementation/internal/upstream/upstream.go [#16] modified: pkg/plugin/implementation/internal/upstream/upstream_test.go [#16] modified: pkg/plugin/implementation/jsonmapper/README.md [#16] modified: pkg/plugin/implementation/schemav2validator/extended_schema.go [#16] modified: pkg/plugin/implementation/schemav2validator/extended_schema_test.go [#16] renamed: pkg/plugin/implementation/oanregistry/README.md -> pkg/plugin/implementation/sunbirdRegistry/README.md [#16] renamed: pkg/plugin/implementation/oanregistry/cmd/plugin.go -> pkg/plugin/implementation/sunbirdRegistry/cmd/plugin.go [#16] renamed: pkg/plugin/implementation/oanregistry/cmd/plugin_test.go -> pkg/plugin/implementation/sunbirdRegistry/cmd/plugin_test.go [#16] renamed: pkg/plugin/implementation/oanregistry/providerrecord.go -> pkg/plugin/implementation/sunbirdRegistry/providerrecord.go [#16] renamed: pkg/plugin/implementation/oanregistry/providerrecord_test.go -> pkg/plugin/implementation/sunbirdRegistry/providerrecord_test.go [#16] renamed: pkg/plugin/implementation/oanregistry/oanregistry.go -> pkg/plugin/implementation/sunbirdRegistry/sunbirdRegistry.go [#16] renamed: pkg/plugin/implementation/oanregistry/oanregistry_test.go -> pkg/plugin/implementation/sunbirdRegistry/sunbirdRegistry_test.go [#16] --- .../agmarknet/mandi-price.select.yaml | 5 ++- ...der-adapter.yaml => provider-adapter.yaml} | 6 +-- install/build-plugins.sh | 2 +- pkg/plugin/definition/mapper.go | 2 +- .../WeatherObservation/mappings_test.go | 2 +- .../capabilitybinding.go} | 16 +++---- .../capabilitybinding_test.go} | 4 +- .../paths.go | 6 +-- .../internal/upstream/upstream.go | 20 ++++----- .../internal/upstream/upstream_test.go | 4 +- .../implementation/jsonmapper/README.md | 2 +- .../schemav2validator/extended_schema.go | 2 +- .../schemav2validator/extended_schema_test.go | 2 +- .../README.md | 6 +-- .../cmd/plugin.go | 42 +++++++++--------- .../cmd/plugin_test.go | 44 +++++++++---------- .../providerrecord.go | 10 ++--- .../providerrecord_test.go | 4 +- .../sunbirdRegistry.go} | 28 ++++++------ .../sunbirdRegistry_test.go} | 12 ++--- 20 files changed, 110 insertions(+), 109 deletions(-) rename config/{oan-provider-adapter.yaml => provider-adapter.yaml} (99%) rename pkg/plugin/implementation/internal/{oanbinding/oanbinding.go => capabilitybinding/capabilitybinding.go} (84%) rename pkg/plugin/implementation/internal/{oanbinding/oanbinding_test.go => capabilitybinding/capabilitybinding_test.go} (99%) rename pkg/plugin/implementation/internal/{oanbinding => capabilitybinding}/paths.go (95%) rename pkg/plugin/implementation/{oanregistry => sunbirdRegistry}/README.md (99%) rename pkg/plugin/implementation/{oanregistry => sunbirdRegistry}/cmd/plugin.go (75%) rename pkg/plugin/implementation/{oanregistry => sunbirdRegistry}/cmd/plugin_test.go (77%) rename pkg/plugin/implementation/{oanregistry => sunbirdRegistry}/providerrecord.go (98%) rename pkg/plugin/implementation/{oanregistry => sunbirdRegistry}/providerrecord_test.go (99%) rename pkg/plugin/implementation/{oanregistry/oanregistry.go => sunbirdRegistry/sunbirdRegistry.go} (96%) rename pkg/plugin/implementation/{oanregistry/oanregistry_test.go => sunbirdRegistry/sunbirdRegistry_test.go} (99%) diff --git a/config/mappings/agmarknet/mandi-price.select.yaml b/config/mappings/agmarknet/mandi-price.select.yaml index d10a0ce8..81e11115 100644 --- a/config/mappings/agmarknet/mandi-price.select.yaml +++ b/config/mappings/agmarknet/mandi-price.select.yaml @@ -57,8 +57,9 @@ required: this guard, the outbound query, and the commodity stamped on each resource. So a caller sending three commodities passed validation, was queried for the first, and got a confident signed answer to a third of - what it asked -- the same failure oanbinding refuses at the commitment - level, where guessing would silently serve part of a request. */ + what it asked -- the same failure capabilitybinding refuses at the + commitment level, where guessing would silently serve part of a + request. */ $exists($ra.supportedCommodities[0].code) and $count($ra.supportedCommodities) = 1 ) diff --git a/config/oan-provider-adapter.yaml b/config/provider-adapter.yaml similarity index 99% rename from config/oan-provider-adapter.yaml rename to config/provider-adapter.yaml index d3694a63..ddd3df18 100644 --- a/config/oan-provider-adapter.yaml +++ b/config/provider-adapter.yaml @@ -19,7 +19,7 @@ # serves. It does NOT hold where a provider lives or how long to wait for it -- # that is the registry's ProviderSchema row, read per request. Repointing a # provider is a registry write, not an edit here and a restart. -appName: "oan-provider-adapter" +appName: "provider-adapter" log: level: debug @@ -80,7 +80,7 @@ modules: plugins: # ------------------------------------------------------------------ - # oanregistry -- the OAN Registry (SunbirdRC) client. + # sunbirdRegistry -- the SunbirdRC registry client. # # Serves both halves of the lookup: the sender's signing key for # validateSign, and the capability call plans the provider steps @@ -89,7 +89,7 @@ modules: # other. # ------------------------------------------------------------------ registry: - id: oanregistry + id: sunbirdRegistry config: # REQUIRED, and the only key with no default. Include the API # version prefix; the plugin appends /{entity}/search. diff --git a/install/build-plugins.sh b/install/build-plugins.sh index 141cb3b0..b7e64256 100755 --- a/install/build-plugins.sh +++ b/install/build-plugins.sh @@ -30,7 +30,7 @@ plugins=( "publisher" "registry" "dediregistry" - "oanregistry" + "sunbirdRegistry" "jsonmapper" "WeatherObservation" "MandiPrice" diff --git a/pkg/plugin/definition/mapper.go b/pkg/plugin/definition/mapper.go index e5c7be95..e1a3790e 100644 --- a/pkg/plugin/definition/mapper.go +++ b/pkg/plugin/definition/mapper.go @@ -17,7 +17,7 @@ const ( // Mapper transforms a document with a mapping fetched from a reference. // -// It exists so that translating between OAN's Beckn payloads and a provider's +// It exists so that translating between the network's Beckn payloads and a provider's // own shape is configuration rather than code: a new provider ships mapping // files, not a new transformation routine. The mapper itself knows nothing // about any provider, and nothing about what a mapping says -- it fetches, diff --git a/pkg/plugin/implementation/WeatherObservation/mappings_test.go b/pkg/plugin/implementation/WeatherObservation/mappings_test.go index b25c04d1..d8688193 100644 --- a/pkg/plugin/implementation/WeatherObservation/mappings_test.go +++ b/pkg/plugin/implementation/WeatherObservation/mappings_test.go @@ -43,7 +43,7 @@ const shippedBindingKey = "mausamgram|openagrinet:WeatherObservation" const shippedMapping = "weather-observation.select.yaml" -// selectRequest is the verbatim /select captured from the OAN network. +// selectRequest is the verbatim /select captured from the network. const selectRequest = `{ "context": { "version": "2.0.0", "action": "select", "networkId": "da.gov.in/vistaar", diff --git a/pkg/plugin/implementation/internal/oanbinding/oanbinding.go b/pkg/plugin/implementation/internal/capabilitybinding/capabilitybinding.go similarity index 84% rename from pkg/plugin/implementation/internal/oanbinding/oanbinding.go rename to pkg/plugin/implementation/internal/capabilitybinding/capabilitybinding.go index b53290ef..e72b1dd7 100644 --- a/pkg/plugin/implementation/internal/oanbinding/oanbinding.go +++ b/pkg/plugin/implementation/internal/capabilitybinding/capabilitybinding.go @@ -1,10 +1,10 @@ -// Package oanbinding derives the capability binding a Beckn request is asking +// Package capabilitybinding derives the capability binding a Beckn request is asking // for, so a provider step can tell whether the request is its work and, if it // is, which registry row describes the call. // // It is shared by every provider step rather than living in one, because the -// binding is a property of the OAN network's payloads and not of any provider. -package oanbinding +// binding is a property of the network's payloads and not of any provider. +package capabilitybinding import ( "encoding/json" @@ -19,7 +19,7 @@ const separator = "|" // ErrNoBinding reports a payload that names no capability binding. It is not a // fault: a request for something else entirely reaches a provider step too, and // the step's answer is to do nothing. -var ErrNoBinding = errors.New("oanbinding: payload names no capability binding") +var ErrNoBinding = errors.New("capabilitybinding: payload names no capability binding") // Binding identifies one provider capability. type Binding struct { @@ -47,7 +47,7 @@ func (b Binding) Key() string { func From(paths Paths, body []byte) (Binding, error) { var payload any if err := json.Unmarshal(body, &payload); err != nil { - return Binding{}, fmt.Errorf("oanbinding: payload could not be read: %w", err) + return Binding{}, fmt.Errorf("capabilitybinding: payload could not be read: %w", err) } // Before distinctness: N commitments naming the SAME provider and type @@ -64,7 +64,7 @@ func From(paths Paths, body []byte) (Binding, error) { // the paragraph above says is refused. if commitments := countAt(payload, paths.ProviderID); commitments > 1 { return Binding{}, fmt.Errorf( - "oanbinding: payload carries %d commitments; one request maps to one call, "+ + "capabilitybinding: payload carries %d commitments; one request maps to one call, "+ "so send them separately rather than have all but the first dropped", commitments) } @@ -76,11 +76,11 @@ func From(paths Paths, body []byte) (Binding, error) { return Binding{}, ErrNoBinding } if len(providers) > 1 { - return Binding{}, fmt.Errorf("oanbinding: payload names %d providers (%s); one request maps to one call", + return Binding{}, fmt.Errorf("capabilitybinding: payload names %d providers (%s); one request maps to one call", len(providers), strings.Join(providers, ", ")) } if len(types) > 1 { - return Binding{}, fmt.Errorf("oanbinding: payload names %d resource types (%s); one request maps to one call", + return Binding{}, fmt.Errorf("capabilitybinding: payload names %d resource types (%s); one request maps to one call", len(types), strings.Join(types, ", ")) } diff --git a/pkg/plugin/implementation/internal/oanbinding/oanbinding_test.go b/pkg/plugin/implementation/internal/capabilitybinding/capabilitybinding_test.go similarity index 99% rename from pkg/plugin/implementation/internal/oanbinding/oanbinding_test.go rename to pkg/plugin/implementation/internal/capabilitybinding/capabilitybinding_test.go index 1a9d0836..43213053 100644 --- a/pkg/plugin/implementation/internal/oanbinding/oanbinding_test.go +++ b/pkg/plugin/implementation/internal/capabilitybinding/capabilitybinding_test.go @@ -1,4 +1,4 @@ -package oanbinding +package capabilitybinding import ( "encoding/json" @@ -7,7 +7,7 @@ import ( "testing" ) -// realSelectPayload is a verbatim /select request captured from the OAN network +// realSelectPayload is a verbatim /select request captured from the network // on 29 Aug 2026. It is the reason this package reads through contract and // commitments rather than off message directly: the design notes showed the // shallower message.offer.provider.id, and the wire does not. diff --git a/pkg/plugin/implementation/internal/oanbinding/paths.go b/pkg/plugin/implementation/internal/capabilitybinding/paths.go similarity index 95% rename from pkg/plugin/implementation/internal/oanbinding/paths.go rename to pkg/plugin/implementation/internal/capabilitybinding/paths.go index 5c47deb2..f1f89a2a 100644 --- a/pkg/plugin/implementation/internal/oanbinding/paths.go +++ b/pkg/plugin/implementation/internal/capabilitybinding/paths.go @@ -1,4 +1,4 @@ -package oanbinding +package capabilitybinding import ( "fmt" @@ -38,11 +38,11 @@ func (p Paths) Validate() error { "capabilityCodeAt": p.CapabilityCode, } { if strings.TrimSpace(path) == "" { - return fmt.Errorf("oanbinding: %s is empty", name) + return fmt.Errorf("capabilitybinding: %s is empty", name) } for _, segment := range strings.Split(path, ".") { if strings.TrimSpace(strings.TrimSuffix(segment, arrayMarker)) == "" { - return fmt.Errorf("oanbinding: %s (%q) has a blank segment", name, path) + return fmt.Errorf("capabilitybinding: %s (%q) has a blank segment", name, path) } } } diff --git a/pkg/plugin/implementation/internal/upstream/upstream.go b/pkg/plugin/implementation/internal/upstream/upstream.go index 40d28fda..2d70ea0d 100644 --- a/pkg/plugin/implementation/internal/upstream/upstream.go +++ b/pkg/plugin/implementation/internal/upstream/upstream.go @@ -33,7 +33,7 @@ import ( "github.com/beckn-one/beckn-onix/pkg/log" "github.com/beckn-one/beckn-onix/pkg/model" "github.com/beckn-one/beckn-onix/pkg/plugin/definition" - "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/oanbinding" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/capabilitybinding" ) // Defaults applied when the registry or the operator leaves a setting out. @@ -164,7 +164,7 @@ type Config struct { // safe for concurrent use. type Step struct { config *Config - paths oanbinding.Paths + paths capabilitybinding.Paths prerequisites Prerequisites registry definition.ProviderRecordLookup mapper definition.Mapper @@ -218,19 +218,19 @@ func New(ctx context.Context, registry definition.ProviderRecordLookup, mapper d // Both halves or neither: overriding one and leaving the other on the default // is a half-configured deployment that would match nothing, and it would do so // silently on every request rather than once at startup. -func bindingPaths(cfg *Config) (oanbinding.Paths, error) { +func bindingPaths(cfg *Config) (capabilitybinding.Paths, error) { if cfg.ProviderIDAt == "" && cfg.CapabilityCodeAt == "" { - return oanbinding.BecknV2, nil + return capabilitybinding.BecknV2, nil } if cfg.ProviderIDAt == "" { - return oanbinding.Paths{}, errors.New("upstream: capabilityCodeAt is set without providerIdAt") + return capabilitybinding.Paths{}, errors.New("upstream: capabilityCodeAt is set without providerIdAt") } if cfg.CapabilityCodeAt == "" { - return oanbinding.Paths{}, errors.New("upstream: providerIdAt is set without capabilityCodeAt") + return capabilitybinding.Paths{}, errors.New("upstream: providerIdAt is set without capabilityCodeAt") } - paths := oanbinding.Paths{ProviderID: cfg.ProviderIDAt, CapabilityCode: cfg.CapabilityCodeAt} + paths := capabilitybinding.Paths{ProviderID: cfg.ProviderIDAt, CapabilityCode: cfg.CapabilityCodeAt} if err := paths.Validate(); err != nil { - return oanbinding.Paths{}, err + return capabilitybinding.Paths{}, err } return paths, nil } @@ -283,8 +283,8 @@ func applyDefaults(cfg *Config) error { // pipeline and each recognises its own work, so adding a provider is one more // entry rather than a change to a routing table. func (s *Step) Run(ctx *model.StepContext) error { - binding, err := oanbinding.From(s.paths, ctx.Body) - if errors.Is(err, oanbinding.ErrNoBinding) { + binding, err := capabilitybinding.From(s.paths, ctx.Body) + if errors.Is(err, capabilitybinding.ErrNoBinding) { return nil } if err != nil { diff --git a/pkg/plugin/implementation/internal/upstream/upstream_test.go b/pkg/plugin/implementation/internal/upstream/upstream_test.go index 33530e33..2e871dea 100644 --- a/pkg/plugin/implementation/internal/upstream/upstream_test.go +++ b/pkg/plugin/implementation/internal/upstream/upstream_test.go @@ -17,7 +17,7 @@ import ( "github.com/beckn-one/beckn-onix/pkg/model" "github.com/beckn-one/beckn-onix/pkg/plugin/definition" - "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/oanbinding" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/capabilitybinding" ) const selectBody = `{ @@ -1086,7 +1086,7 @@ func TestNewUsesTheBecknConventionByDefault(t *testing.T) { t.Parallel() step := newStep(t, &stubRegistry{}, &stubMapper{}) - if step.paths != oanbinding.BecknV2 { + if step.paths != capabilitybinding.BecknV2 { t.Errorf("paths = %+v, want the Beckn v2 convention", step.paths) } } diff --git a/pkg/plugin/implementation/jsonmapper/README.md b/pkg/plugin/implementation/jsonmapper/README.md index 1f1d5913..e837331a 100644 --- a/pkg/plugin/implementation/jsonmapper/README.md +++ b/pkg/plugin/implementation/jsonmapper/README.md @@ -13,7 +13,7 @@ what a mapping says, and nothing about the payloads passing through. Anything specific to a network or a provider belongs in the caller, which is what lets one mapper serve all of them. -Its first caller is the OAN provider flow, where it translates between Beckn +Its first caller is the provider flow, where it translates between Beckn payloads and each provider's own request and response shapes -- so adding a provider is one mapping file and a registry row rather than another transformation routine. diff --git a/pkg/plugin/implementation/schemav2validator/extended_schema.go b/pkg/plugin/implementation/schemav2validator/extended_schema.go index 654ba2d7..cf9cc91f 100644 --- a/pkg/plugin/implementation/schemav2validator/extended_schema.go +++ b/pkg/plugin/implementation/schemav2validator/extended_schema.go @@ -531,7 +531,7 @@ func stripUnaccountedJSONLDKeys(schema *openapi3.SchemaRef, data map[string]inte // listed as required, anywhere in a schema's composition tree. // // allOf, anyOf, oneOf and the then/else branches can each introduce a property, -// so all of them are walked -- the OAN packs declare @type one level down, in +// so all of them are walked -- the capability packs declare @type one level down, in // allOf. "not" is skipped because naming a property there forbids it rather // than permitting it, and "if" is skipped because it only selects a branch. // seen guards against schemas that reference themselves. diff --git a/pkg/plugin/implementation/schemav2validator/extended_schema_test.go b/pkg/plugin/implementation/schemav2validator/extended_schema_test.go index e38325b1..8f7e378c 100644 --- a/pkg/plugin/implementation/schemav2validator/extended_schema_test.go +++ b/pkg/plugin/implementation/schemav2validator/extended_schema_test.go @@ -1379,7 +1379,7 @@ func TestLoadSchemaFromPath_TTLExpiry_FetchesFresh(t *testing.T) { assert.Equal(t, "Schema v2", doc2.Info.Title, "expected v2 after TTL expiry — global URIMapCache not bypassed") } -// packStyleSchema mirrors how the OAN schema packs are shaped: the capability +// packStyleSchema mirrors how the capability schema packs are shaped: the capability // declares @type one level down in allOf and lists it as required, and nothing // closes the object with additionalProperties:false. const packStyleSchema = `openapi: 3.1.0 diff --git a/pkg/plugin/implementation/oanregistry/README.md b/pkg/plugin/implementation/sunbirdRegistry/README.md similarity index 99% rename from pkg/plugin/implementation/oanregistry/README.md rename to pkg/plugin/implementation/sunbirdRegistry/README.md index cbce9a81..459d51fe 100644 --- a/pkg/plugin/implementation/oanregistry/README.md +++ b/pkg/plugin/implementation/sunbirdRegistry/README.md @@ -1,6 +1,6 @@ -# OAN Registry Plugin +# SunbirdRC Registry Plugin -A **registry type plugin** for Beckn-ONIX that reads the OAN Registry, a +A **registry type plugin** for Beckn-ONIX that reads a SunbirdRC registry, a [SunbirdRC](https://docs.sunbirdrc.dev/) deployment. ## Overview @@ -31,7 +31,7 @@ request spends waiting before it can even be rejected. ```yaml registry: - id: oanregistry + id: sunbirdRegistry config: url: http://registry:8081/api/v1 entity: Participant diff --git a/pkg/plugin/implementation/oanregistry/cmd/plugin.go b/pkg/plugin/implementation/sunbirdRegistry/cmd/plugin.go similarity index 75% rename from pkg/plugin/implementation/oanregistry/cmd/plugin.go rename to pkg/plugin/implementation/sunbirdRegistry/cmd/plugin.go index 461432a6..5992bf4b 100644 --- a/pkg/plugin/implementation/oanregistry/cmd/plugin.go +++ b/pkg/plugin/implementation/sunbirdRegistry/cmd/plugin.go @@ -9,33 +9,33 @@ import ( "github.com/beckn-one/beckn-onix/pkg/log" "github.com/beckn-one/beckn-onix/pkg/plugin/definition" - "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/oanregistry" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/sunbirdRegistry" ) // Defaults for settings an operator leaves out. Only parseConfig can tell // "absent" from "explicitly zero" -- retry_max of 0 is a legitimate "do not // retry" -- so they are applied here. The values themselves live in the -// oanregistry package so there is exactly one place to change them. +// sunbirdRegistry package so there is exactly one place to change them. const ( - defaultEntity = oanregistry.DefaultEntity - defaultProviderEntity = oanregistry.DefaultProviderEntity - defaultTimeout = oanregistry.DefaultTimeoutSeconds - defaultRetryMax = oanregistry.DefaultRetryMax - defaultRetryWaitMin = oanregistry.DefaultRetryWaitMin - defaultRetryWaitMax = oanregistry.DefaultRetryWaitMax + defaultEntity = sunbirdRegistry.DefaultEntity + defaultProviderEntity = sunbirdRegistry.DefaultProviderEntity + defaultTimeout = sunbirdRegistry.DefaultTimeoutSeconds + defaultRetryMax = sunbirdRegistry.DefaultRetryMax + defaultRetryWaitMin = sunbirdRegistry.DefaultRetryWaitMin + defaultRetryWaitMax = sunbirdRegistry.DefaultRetryWaitMax ) -// oanRegistryProvider implements the RegistryLookupProvider interface for the -// OAN registry plugin. -type oanRegistryProvider struct{} +// sunbirdRegistryProvider implements the RegistryLookupProvider interface for the +// registry plugin. +type sunbirdRegistryProvider struct{} -// newOANRegistryFunc creates a new OAN registry client. Indirected for tests. -var newOANRegistryFunc = oanregistry.New +// newSunbirdRegistryFunc creates a new registry client. Indirected for tests. +var newSunbirdRegistryFunc = sunbirdRegistry.New -// parseConfig parses the configuration map into an oanregistry.Config, starting +// parseConfig parses the configuration map into an sunbirdRegistry.Config, starting // from the defaults and overriding whatever the operator supplied. -func (o oanRegistryProvider) parseConfig(config map[string]string) (*oanregistry.Config, error) { - cfg := &oanregistry.Config{ +func (o sunbirdRegistryProvider) parseConfig(config map[string]string) (*sunbirdRegistry.Config, error) { + cfg := &sunbirdRegistry.Config{ URL: config["url"], Entity: defaultEntity, ProviderEntity: defaultProviderEntity, @@ -135,8 +135,8 @@ func (o oanRegistryProvider) parseConfig(config map[string]string) (*oanregistry return cfg, nil } -// New creates a new OAN registry plugin instance. -func (o oanRegistryProvider) New(ctx context.Context, cache definition.Cache, config map[string]string) (definition.RegistryLookup, func() error, error) { +// New creates a new registry plugin instance. +func (o sunbirdRegistryProvider) New(ctx context.Context, cache definition.Cache, config map[string]string) (definition.RegistryLookup, func() error, error) { if ctx == nil { return nil, nil, errors.New("context cannot be nil") } @@ -144,12 +144,12 @@ func (o oanRegistryProvider) New(ctx context.Context, cache definition.Cache, co cfg, err := o.parseConfig(config) if err != nil { log.Errorf(ctx, err, "Failed to parse OAN registry configuration") - return nil, nil, fmt.Errorf("failed to parse oan registry configuration: %w", err) + return nil, nil, fmt.Errorf("failed to parse registry configuration: %w", err) } log.Debugf(ctx, "OAN registry config mapped: %+v", cfg) - client, closer, err := newOANRegistryFunc(ctx, cache, cfg) + client, closer, err := newSunbirdRegistryFunc(ctx, cache, cfg) if err != nil { log.Errorf(ctx, err, "Failed to create OAN registry instance") return nil, nil, err @@ -160,4 +160,4 @@ func (o oanRegistryProvider) New(ctx context.Context, cache definition.Cache, co } // Provider is the exported plugin instance. -var Provider = oanRegistryProvider{} +var Provider = sunbirdRegistryProvider{} diff --git a/pkg/plugin/implementation/oanregistry/cmd/plugin_test.go b/pkg/plugin/implementation/sunbirdRegistry/cmd/plugin_test.go similarity index 77% rename from pkg/plugin/implementation/oanregistry/cmd/plugin_test.go rename to pkg/plugin/implementation/sunbirdRegistry/cmd/plugin_test.go index 8d36d67a..fb2dfec5 100644 --- a/pkg/plugin/implementation/oanregistry/cmd/plugin_test.go +++ b/pkg/plugin/implementation/sunbirdRegistry/cmd/plugin_test.go @@ -9,11 +9,11 @@ import ( "time" "github.com/beckn-one/beckn-onix/pkg/plugin/definition" - "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/oanregistry" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/sunbirdRegistry" ) -func defaultConfig() *oanregistry.Config { - return &oanregistry.Config{ +func defaultConfig() *sunbirdRegistry.Config { + return &sunbirdRegistry.Config{ Entity: defaultEntity, ProviderEntity: defaultProviderEntity, Timeout: defaultTimeout, @@ -26,7 +26,7 @@ func defaultConfig() *oanregistry.Config { func TestParseConfig(t *testing.T) { t.Parallel() - withDefaults := func(apply func(*oanregistry.Config)) *oanregistry.Config { + withDefaults := func(apply func(*sunbirdRegistry.Config)) *sunbirdRegistry.Config { cfg := defaultConfig() apply(cfg) return cfg @@ -35,13 +35,13 @@ func TestParseConfig(t *testing.T) { testCases := []struct { name string config map[string]string - expected *oanregistry.Config + expected *sunbirdRegistry.Config expectedErr string }{ { name: "applies defaults when only a URL is given", config: map[string]string{"url": "http://registry:8081/api/v1"}, - expected: withDefaults(func(c *oanregistry.Config) { c.URL = "http://registry:8081/api/v1" }), + expected: withDefaults(func(c *sunbirdRegistry.Config) { c.URL = "http://registry:8081/api/v1" }), }, { name: "reads every supported setting", @@ -54,7 +54,7 @@ func TestParseConfig(t *testing.T) { "retry_wait_min": "200ms", "retry_wait_max": "1s", }, - expected: &oanregistry.Config{ + expected: &sunbirdRegistry.Config{ URL: "http://registry:8081/api/v1", Entity: "Subscriber", ProviderEntity: defaultProviderEntity, @@ -71,7 +71,7 @@ func TestParseConfig(t *testing.T) { "url": "http://registry:8081", "providerEntity": "ProviderCapability", }, - expected: withDefaults(func(c *oanregistry.Config) { + expected: withDefaults(func(c *sunbirdRegistry.Config) { c.URL = "http://registry:8081" c.ProviderEntity = "ProviderCapability" }), @@ -82,7 +82,7 @@ func TestParseConfig(t *testing.T) { "url": "http://registry:8081", "providerEntity": "", }, - expected: withDefaults(func(c *oanregistry.Config) { + expected: withDefaults(func(c *sunbirdRegistry.Config) { c.URL = "http://registry:8081" }), }, @@ -91,18 +91,18 @@ func TestParseConfig(t *testing.T) { // participant keeps verifying. name: "leaves caching disabled when no TTL is set", config: map[string]string{"url": "http://registry:8081"}, - expected: withDefaults(func(c *oanregistry.Config) { c.URL = "http://registry:8081" }), + expected: withDefaults(func(c *sunbirdRegistry.Config) { c.URL = "http://registry:8081" }), }, { // Distinct from "unset", which yields the default of 1. name: "honours an explicit retry_max of zero", config: map[string]string{"url": "http://registry:8081", "retry_max": "0"}, - expected: withDefaults(func(c *oanregistry.Config) { c.URL = "http://registry:8081"; c.RetryMax = 0 }), + expected: withDefaults(func(c *sunbirdRegistry.Config) { c.URL = "http://registry:8081"; c.RetryMax = 0 }), }, { name: "ignores empty values and keeps the defaults", config: map[string]string{"url": "http://registry:8081", "entity": "", "timeout": ""}, - expected: withDefaults(func(c *oanregistry.Config) { c.URL = "http://registry:8081" }), + expected: withDefaults(func(c *sunbirdRegistry.Config) { c.URL = "http://registry:8081" }), }, { name: "rejects a non-numeric timeout", @@ -144,7 +144,7 @@ func TestParseConfig(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - got, err := oanRegistryProvider{}.parseConfig(tc.config) + got, err := sunbirdRegistryProvider{}.parseConfig(tc.config) if tc.expectedErr != "" { if err == nil { @@ -173,7 +173,7 @@ func TestNew(t *testing.T) { t.Parallel() //nolint:staticcheck // deliberately passing a nil context to assert the guard. - _, _, err := oanRegistryProvider{}.New(nil, nil, map[string]string{"url": "http://registry:8081"}) + _, _, err := sunbirdRegistryProvider{}.New(nil, nil, map[string]string{"url": "http://registry:8081"}) if err == nil { t.Fatal("expected an error for a nil context, got none") } @@ -182,7 +182,7 @@ func TestNew(t *testing.T) { t.Run("rejects a missing URL", func(t *testing.T) { t.Parallel() - _, _, err := oanRegistryProvider{}.New(context.Background(), nil, map[string]string{}) + _, _, err := sunbirdRegistryProvider{}.New(context.Background(), nil, map[string]string{}) if err == nil { t.Fatal("expected an error for a missing URL, got none") } @@ -191,7 +191,7 @@ func TestNew(t *testing.T) { t.Run("rejects an unparseable config", func(t *testing.T) { t.Parallel() - _, _, err := oanRegistryProvider{}.New(context.Background(), nil, map[string]string{ + _, _, err := sunbirdRegistryProvider{}.New(context.Background(), nil, map[string]string{ "url": "http://registry:8081", "timeout": "soon", }) @@ -203,7 +203,7 @@ func TestNew(t *testing.T) { t.Run("builds a client from a valid config", func(t *testing.T) { t.Parallel() - client, closer, err := oanRegistryProvider{}.New(context.Background(), nil, map[string]string{ + client, closer, err := sunbirdRegistryProvider{}.New(context.Background(), nil, map[string]string{ "url": "http://registry:8081/api/v1", }) if err != nil { @@ -220,20 +220,20 @@ func TestNew(t *testing.T) { } }) - // Deliberately NOT parallel: this swaps the package-level newOANRegistryFunc, + // Deliberately NOT parallel: this swaps the package-level newSunbirdRegistryFunc, // so running it alongside its parallel siblings would race on that variable. // Go never schedules a non-parallel subtest concurrently with parallel ones, // which is what makes this safe -- do not add t.Parallel() "for consistency". t.Run("propagates a client construction failure", func(t *testing.T) { - original := newOANRegistryFunc - t.Cleanup(func() { newOANRegistryFunc = original }) + original := newSunbirdRegistryFunc + t.Cleanup(func() { newSunbirdRegistryFunc = original }) wantErr := errors.New("boom") - newOANRegistryFunc = func(context.Context, definition.Cache, *oanregistry.Config) (*oanregistry.Client, func() error, error) { + newSunbirdRegistryFunc = func(context.Context, definition.Cache, *sunbirdRegistry.Config) (*sunbirdRegistry.Client, func() error, error) { return nil, nil, wantErr } - _, _, err := oanRegistryProvider{}.New(context.Background(), nil, map[string]string{"url": "http://registry:8081"}) + _, _, err := sunbirdRegistryProvider{}.New(context.Background(), nil, map[string]string{"url": "http://registry:8081"}) if !errors.Is(err, wantErr) { t.Fatalf("expected the underlying error to be propagated, got: %v", err) } diff --git a/pkg/plugin/implementation/oanregistry/providerrecord.go b/pkg/plugin/implementation/sunbirdRegistry/providerrecord.go similarity index 98% rename from pkg/plugin/implementation/oanregistry/providerrecord.go rename to pkg/plugin/implementation/sunbirdRegistry/providerrecord.go index cf3dd5a6..03e621d0 100644 --- a/pkg/plugin/implementation/oanregistry/providerrecord.go +++ b/pkg/plugin/implementation/sunbirdRegistry/providerrecord.go @@ -1,10 +1,10 @@ -package oanregistry +package sunbirdRegistry // providerrecord.go resolves a capability binding into a call plan: what to // call, how to call it, and which mappings translate in and out. // -// This is the second thing the OAN registry is asked for, and it is a different -// question from the signing-key lookup in oanregistry.go. That one asks "who +// This is the second thing the registry is asked for, and it is a different +// question from the signing-key lookup in sunbirdRegistry.go. That one asks "who // sent this", keyed by an inbound Authorization header. This one asks "who do I // call next", keyed by a binding taken from the request body. Different subject, // different cache, different meaning of failure -- so they share transport and @@ -95,7 +95,7 @@ func searchURLFor(baseURL, entity string) string { func (c *Client) ProviderRecord(ctx context.Context, bindingKey string) (*model.ProviderRecord, error) { start := time.Now() tracer := otel.Tracer(telemetry.ScopeName, trace.WithInstrumentationVersion(telemetry.ScopeVersion)) - ctx, span := tracer.Start(ctx, "oan registry provider record") + ctx, span := tracer.Start(ctx, "registry provider record") defer span.End() if bindingKey == "" { @@ -373,7 +373,7 @@ func decodeRecords[T any](body []byte) ([]T, error) { // one cache but have different subjects and lifetimes, and a collision would // serve one as the other. func providerRecordCacheKey(bindingKey string) string { - return "oan_provider_" + bindingKey + return "registry_provider_" + bindingKey } func (c *Client) cachedProviderRecord(ctx context.Context, tracer trace.Tracer, key string) (*model.ProviderRecord, bool) { diff --git a/pkg/plugin/implementation/oanregistry/providerrecord_test.go b/pkg/plugin/implementation/sunbirdRegistry/providerrecord_test.go similarity index 99% rename from pkg/plugin/implementation/oanregistry/providerrecord_test.go rename to pkg/plugin/implementation/sunbirdRegistry/providerrecord_test.go index 2f83315f..aab2c4c8 100644 --- a/pkg/plugin/implementation/oanregistry/providerrecord_test.go +++ b/pkg/plugin/implementation/sunbirdRegistry/providerrecord_test.go @@ -1,4 +1,4 @@ -package oanregistry +package sunbirdRegistry import ( "context" @@ -591,7 +591,7 @@ func TestProviderRecordCacheKeyIsDistinctFromTheKeyLookupCacheKey(t *testing.T) if _, err := resolvePlan(t, client); err != nil { t.Fatalf("ProviderRecord() returned an unexpected error: %v", err) } - if strings.HasPrefix(cache.setKey, "oan_lookup_") { + if strings.HasPrefix(cache.setKey, "registry_lookup_") { t.Errorf("provider plan cache key %q shares the signing-key namespace", cache.setKey) } } diff --git a/pkg/plugin/implementation/oanregistry/oanregistry.go b/pkg/plugin/implementation/sunbirdRegistry/sunbirdRegistry.go similarity index 96% rename from pkg/plugin/implementation/oanregistry/oanregistry.go rename to pkg/plugin/implementation/sunbirdRegistry/sunbirdRegistry.go index 950829cc..0d598eb5 100644 --- a/pkg/plugin/implementation/oanregistry/oanregistry.go +++ b/pkg/plugin/implementation/sunbirdRegistry/sunbirdRegistry.go @@ -1,9 +1,9 @@ -// Package oanregistry resolves participant signing keys from the OAN Registry -// (a SunbirdRC deployment) so inbound Beckn signatures can be verified. +// Package sunbirdRegistry resolves participant signing keys from a SunbirdRC +// registry so inbound Beckn signatures can be verified. // // It implements definition.RegistryLookup only. Onboarding, key publication and // status changes all happen through the registry's own Participant APIs. -package oanregistry +package sunbirdRegistry import ( "context" @@ -129,13 +129,13 @@ func classify(err error) string { } const ( - pluginID = "oanregistry" + pluginID = "sunbirdRegistry" pluginType = "registry" operationLookup = "lookup" operationProviderRecord = "provider_record" ) -// Config holds configuration parameters for the OAN registry client. +// Config holds configuration parameters for the registry client. type Config struct { // URL is the registry base including any API version prefix, // e.g. "http://registry:8081/api/v1". @@ -156,7 +156,7 @@ type Config struct { MaxResponseBytes int64 `yaml:"maxResponseBytes" json:"maxResponseBytes"` } -// Client resolves participants from the OAN registry. It is safe for concurrent +// Client resolves participants from the registry. It is safe for concurrent // use: every field is set once in New and never mutated afterwards. type Client struct { searchURL string @@ -232,23 +232,23 @@ type searchRequest struct { Filters map[string]eqFilter `json:"filters"` } -// validate checks if the provided OAN registry configuration is valid. +// validate checks if the provided registry configuration is valid. func validate(cfg *Config) error { if cfg == nil { - return fmt.Errorf("oan registry config cannot be nil") + return fmt.Errorf("registry config cannot be nil") } if cfg.URL == "" { - return fmt.Errorf("oan registry URL cannot be empty") + return fmt.Errorf("registry URL cannot be empty") } // url.Parse accepts almost anything, so check the parts that actually have // to be there. Catching "registry:8081" (no scheme) at startup is far // cheaper than watching every lookup fail once traffic arrives. parsed, err := url.Parse(cfg.URL) if err != nil { - return fmt.Errorf("invalid oan registry URL %q: %w", cfg.URL, err) + return fmt.Errorf("invalid registry URL %q: %w", cfg.URL, err) } if parsed.Scheme == "" || parsed.Host == "" { - return fmt.Errorf("oan registry URL %q must include a scheme and host, e.g. http://:/api/v1", cfg.URL) + return fmt.Errorf("registry URL %q must include a scheme and host, e.g. http://:/api/v1", cfg.URL) } return nil } @@ -359,7 +359,7 @@ func New(ctx context.Context, cache definition.Cache, cfg *Config) (*Client, fun func (c *Client) Lookup(ctx context.Context, req *model.Subscription) ([]model.Subscription, error) { start := time.Now() tracer := otel.Tracer(telemetry.ScopeName, trace.WithInstrumentationVersion(telemetry.ScopeVersion)) - ctx, span := tracer.Start(ctx, "oan registry lookup") + ctx, span := tracer.Start(ctx, "registry lookup") defer span.End() // M2: an empty key id would match any record whose OSID is absent. Unreachable @@ -371,7 +371,7 @@ func (c *Client) Lookup(ctx context.Context, req *model.Subscription) ([]model.S return nil, nil } - cacheKey := fmt.Sprintf("oan_lookup_%s_%s", req.SubscriberID, req.KeyID) + cacheKey := fmt.Sprintf("registry_lookup_%s_%s", req.SubscriberID, req.KeyID) if cached, ok := c.cached(ctx, tracer, cacheKey); ok { log.Debugf(ctx, "OAN registry lookup cache hit for key: %s", cacheKey) span.SetAttributes(telemetry.AttrErrorType.String(outcomeCacheHit)) @@ -547,7 +547,7 @@ func toSubscription(p participant, k key, status string) model.Subscription { validFrom, _ := parseTime(k.ValidFrom) validUntil, _ := parseTime(k.ValidUntil) - // Domain is absent from the OAN record and so is left unset. Nothing on the + // Domain is absent from the registry record and so is left unset. Nothing on the // signature-validation path reads it. return model.Subscription{ Subscriber: model.Subscriber{ diff --git a/pkg/plugin/implementation/oanregistry/oanregistry_test.go b/pkg/plugin/implementation/sunbirdRegistry/sunbirdRegistry_test.go similarity index 99% rename from pkg/plugin/implementation/oanregistry/oanregistry_test.go rename to pkg/plugin/implementation/sunbirdRegistry/sunbirdRegistry_test.go index bc6f438a..ddbe9711 100644 --- a/pkg/plugin/implementation/oanregistry/oanregistry_test.go +++ b/pkg/plugin/implementation/sunbirdRegistry/sunbirdRegistry_test.go @@ -1,4 +1,4 @@ -package oanregistry +package sunbirdRegistry import ( "context" @@ -144,12 +144,12 @@ func TestValidate(t *testing.T) { { name: "should return error for nil config", config: nil, - expectedErr: "oan registry config cannot be nil", + expectedErr: "registry config cannot be nil", }, { name: "should return error for empty URL", config: &Config{URL: ""}, - expectedErr: "oan registry URL cannot be empty", + expectedErr: "registry URL cannot be empty", }, { name: "should succeed for valid config", @@ -1169,7 +1169,7 @@ func TestLookupCachesUsableResults(t *testing.T) { if cache.setTTL != ttl { t.Errorf("expected the configured TTL %v, got %v", ttl, cache.setTTL) } - if expected := fmt.Sprintf("oan_lookup_%s_%s", testParticipantID, testOSID); cache.setKey != expected { + if expected := fmt.Sprintf("registry_lookup_%s_%s", testParticipantID, testOSID); cache.setKey != expected { t.Errorf("expected cache key %q, got %q", expected, cache.setKey) } @@ -1358,7 +1358,7 @@ func assertOutcomeAttribute(t *testing.T, m metricdata.Metrics, outcome string) } // TestLookupAgainstCapturedRegistryResponse runs the plugin against a verbatim -// response captured from the real OAN registry on 31 Aug 2026, reformatted for +// response captured from a live registry on 31 Aug 2026, reformatted for // readability with field order and values untouched. // // It pins the deployed shape: the data envelope, one flat level with the keys @@ -1482,7 +1482,7 @@ func TestLookupAgainstCapturedRegistryResponse(t *testing.T) { } // TestLookupAgainstCurrentRegistryResponse runs the plugin against a verbatim -// response captured from an OAN registry on 2 Sep 2026, after the Participant +// response captured from a live registry on 2 Sep 2026, after the Participant // schema dropped three things from a published key. // // It pins the shape a registry writes TODAY, and every difference from the From 78d42b9303f691057eb2b94c1d000e4c6271b837 Mon Sep 17 00:00:00 2001 From: ameersohel45 Date: Mon, 7 Sep 2026 18:02:45 +0530 Subject: [PATCH 06/10] docs(config): drop the network name from the reference config's comments [#16] The plugins are generic and their comments should read that way: jsonmapper knows nothing about any provider, and the validator's if/then gap applies to any pack, not to one network's. This commit used to also placeholder the subscriberId. That work is now in the base branch, so what is left here is the comment wording alone, and the message says so rather than claiming a change that is no longer in the diff. --- config/provider-adapter.yaml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/config/provider-adapter.yaml b/config/provider-adapter.yaml index ddd3df18..e892811b 100644 --- a/config/provider-adapter.yaml +++ b/config/provider-adapter.yaml @@ -1,4 +1,4 @@ -# OAN provider adapter. +# Provider adapter. # # Serves the Beckn actions synchronously: verifies the sender, resolves the # capability's call plan from the registry, calls the provider, and answers with @@ -177,9 +177,9 @@ modules: # # What it does NOT: if/then/else. The validator library parses those # keywords but never evaluates them, so a pack's conditional rules -- - # in the OAN packs, everything predicated on informationMode -- are - # not checked. Worth knowing before treating a pass here as full - # conformance to a pack. + # in the capability packs, everything predicated on informationMode + # -- are not checked. Worth knowing before treating a pass here as + # full conformance to a pack. # ------------------------------------------------------------------ schemaValidator: id: schemav2validator @@ -217,7 +217,7 @@ modules: # ------------------------------------------------------------------ # jsonmapper -- the JSONata mapper. # - # Generic, and named for what it is rather than for OAN: it knows + # Generic, and named for what it is rather than for a network: it knows # nothing about any provider. It fetches whatever URL the registry's # mappings field names, compiles the JSONata, caches the compiled # form, and runs it in both directions. From 64a1de28da78f91846c06dd1a29599ac53b9fe58 Mon Sep 17 00:00:00 2001 From: ameersohel45 Date: Tue, 8 Sep 2026 00:55:38 +0530 Subject: [PATCH 07/10] fix(schemav2validator): read only http and https for a payload-chosen schema [#16] The allowlist was consulted once, on the top-level @context. Every document fetched to resolve that document's $refs met no check at all, and freshReadFromURI falls through to os.ReadFile for any scheme but http and https. So a $ref of "file:///etc/passwd" -- or a bare path, which parses with no scheme -- was an instruction from the network to open this container's disk and parse it as a schema. Measured: one capability pack pulls 15 documents, so 14 of the 15 reads were unchecked. A loader on this path now refuses anything but http and https, for the entry document and every $ref under it. The base spec loader keeps the file fallthrough deliberately: its location is operator-configured, where a local file is the point. localSchema mode keeps it for the same reason. This does not restrict which HOSTS a $ref may reach, and that is deliberate rather than missed: the packs $ref two external spec hosts, so enforcing the allowlist on refs needs those named in it as well, or no pack loads at all. TestValidateReferencedObject_AllowsARefToAHostOutsideTheAllowlist pins that, so the wider change cannot be made without noticing. Two existing tests asserted the old behaviour rather than a requirement, and say the new one now; several others used a local temp file as a fixture and either load it in operator mode or serve it over http, which is what production does anyway. --- .../schemav2validator/extended_schema.go | 30 ++ .../schemav2validator/extended_schema_test.go | 385 +++++++++++++----- 2 files changed, 303 insertions(+), 112 deletions(-) diff --git a/pkg/plugin/implementation/schemav2validator/extended_schema.go b/pkg/plugin/implementation/schemav2validator/extended_schema.go index cf9cc91f..2fdcb614 100644 --- a/pkg/plugin/implementation/schemav2validator/extended_schema.go +++ b/pkg/plugin/implementation/schemav2validator/extended_schema.go @@ -323,6 +323,15 @@ func (c *schemaCache) loadSchemaFromPath(ctx context.Context, schemaPath string, loader := newFreshLoader() loader.Context = ctx + if !localSchema { + // The schema location on this path is derived from a payload's + // @context, so every read it causes is network-directed. Installed + // here rather than at the one @context check because that check runs + // once, on the entry document: the $refs inside whatever comes back + // are resolved by the loader and meet no check at all. One pack pulls + // 15 documents across 3 hosts, so this is the majority of the reads. + loader.ReadFromURIFunc = payloadDirectedReader + } var doc *openapi3.T var err error @@ -409,6 +418,27 @@ func (c *schemaCache) loadSchemaFromPath(ctx context.Context, schemaPath string, return doc, nil } +// payloadDirectedReader reads a schema document for a location that a payload +// chose, refusing any scheme but http and https. +// +// freshReadFromURI falls through to os.ReadFile for every other scheme, so +// without this a $ref of "file:///etc/passwd" -- or a bare path, which parses +// with no scheme at all -- is an instruction from the network to read this +// container's disk and parse it as a schema. The base spec loader keeps that +// fallthrough deliberately: its location is operator-configured, where a local +// file is the point. Here it never is. +// +// This does NOT restrict which hosts may be reached; isAllowedDomain still +// guards only the entry @context. Enforcing the allowlist here as well is the +// right shape, but the packs $ref two external spec hosts, so it needs those +// named in the allowlist or no pack loads at all. +func payloadDirectedReader(loader *openapi3.Loader, u *url.URL) ([]byte, error) { + if u.Scheme != "http" && u.Scheme != "https" { + return nil, fmt.Errorf("refusing to read schema from %q: only http and https are read for a location a payload chose", u.String()) + } + return freshReadFromURI(loader, u) +} + // findReferencedObjects recursively finds domain-specific objects with @context. func findReferencedObjects(data interface{}, path string) []referencedObject { var results []referencedObject diff --git a/pkg/plugin/implementation/schemav2validator/extended_schema_test.go b/pkg/plugin/implementation/schemav2validator/extended_schema_test.go index 8f7e378c..39beebff 100644 --- a/pkg/plugin/implementation/schemav2validator/extended_schema_test.go +++ b/pkg/plugin/implementation/schemav2validator/extended_schema_test.go @@ -10,6 +10,7 @@ import ( "path/filepath" "reflect" "sort" + "strings" "sync/atomic" "testing" "time" @@ -156,10 +157,10 @@ func TestHashURL(t *testing.T) { t.Run(tt.name, func(t *testing.T) { hash1 := hashURL(tt.url) hash2 := hashURL(tt.url) - + // Same URL should produce same hash assert.Equal(t, hash1, hash2) - + // Hash should be 64 characters (SHA256 hex) assert.Equal(t, 64, len(hash1)) }) @@ -247,23 +248,23 @@ func TestNewSchemaCache(t *testing.T) { func TestSchemaCache_GetSet(t *testing.T) { cache := newSchemaCache(10) - + // Create a simple schema doc doc := &openapi3.T{ OpenAPI: "3.1.0", } - + urlHash := hashURL("https://example.com/schema.yaml") ttl := 1 * time.Hour - + // Test Set cache.set(urlHash, doc, ttl) - + // Test Get - should find it retrieved, found := cache.get(urlHash) assert.True(t, found) assert.Equal(t, doc, retrieved) - + // Test Get - non-existent key _, found = cache.get("non-existent-hash") assert.False(t, found) @@ -271,28 +272,28 @@ func TestSchemaCache_GetSet(t *testing.T) { func TestSchemaCache_LRUEviction(t *testing.T) { cache := newSchemaCache(2) // Small cache for testing - + doc1 := &openapi3.T{OpenAPI: "3.1.0"} doc2 := &openapi3.T{OpenAPI: "3.1.1"} doc3 := &openapi3.T{OpenAPI: "3.1.2"} - + ttl := 1 * time.Hour - + // Add first two items cache.set("hash1", doc1, ttl) cache.set("hash2", doc2, ttl) - + // Access first item to make it more recent cache.get("hash1") - + // Add third item - should evict hash2 (least recently used) cache.set("hash3", doc3, ttl) - + // Verify hash1 and hash3 exist, hash2 was evicted _, found1 := cache.get("hash1") _, found2 := cache.get("hash2") _, found3 := cache.get("hash3") - + assert.True(t, found1, "hash1 should exist (recently accessed)") assert.False(t, found2, "hash2 should be evicted (LRU)") assert.True(t, found3, "hash3 should exist (just added)") @@ -300,20 +301,20 @@ func TestSchemaCache_LRUEviction(t *testing.T) { func TestSchemaCache_TTLExpiry(t *testing.T) { cache := newSchemaCache(10) - + doc := &openapi3.T{OpenAPI: "3.1.0"} urlHash := "test-hash" - + // Set with very short TTL cache.set(urlHash, doc, 1*time.Millisecond) - + // Should be found immediately _, found := cache.get(urlHash) assert.True(t, found) - + // Wait for expiry time.Sleep(10 * time.Millisecond) - + // Should not be found after expiry _, found = cache.get(urlHash) assert.False(t, found) @@ -321,23 +322,23 @@ func TestSchemaCache_TTLExpiry(t *testing.T) { func TestSchemaCache_CleanupExpired(t *testing.T) { cache := newSchemaCache(10) - + doc := &openapi3.T{OpenAPI: "3.1.0"} - + // Add items with short TTL cache.set("hash1", doc, 1*time.Millisecond) cache.set("hash2", doc, 1*time.Millisecond) cache.set("hash3", doc, 1*time.Hour) // This one won't expire - + // Wait for expiry time.Sleep(10 * time.Millisecond) - + // Cleanup expired count := cache.cleanupExpired() - + // Should have cleaned up 2 expired items assert.Equal(t, 2, count) - + // Verify only hash3 remains cache.mu.RLock() assert.Equal(t, 1, len(cache.schemas)) @@ -448,7 +449,7 @@ func TestFindReferencedObjects_PathBuilding(t *testing.T) { } objects := findReferencedObjects(data, "message") - + assert.Equal(t, 1, len(objects)) assert.Equal(t, "message.order.beckn:orderItems[0].beckn:acceptedOffer.beckn:offerAttributes", objects[0].Path) assert.Equal(t, "ChargingOffer", objects[0].Type) @@ -459,11 +460,11 @@ func TestFindReferencedObjects_PathBuilding(t *testing.T) { func TestLoadSchemaFromPath_LocalFile(t *testing.T) { cache := newSchemaCache(10) ctx := context.Background() - + tmpFile, err := os.CreateTemp("", "test-schema-*.yaml") assert.NoError(t, err) defer os.Remove(tmpFile.Name()) - + schemaContent := `openapi: 3.1.0 info: title: Test Schema @@ -475,58 +476,74 @@ components: properties: field1: type: string` - + _, err = tmpFile.Write([]byte(schemaContent)) assert.NoError(t, err) tmpFile.Close() - + + // localSchema=false means the location came from a payload's @context -- + // the only way the production caller passes it. A local file is not + // something the network may ask this process to open, so it is refused. doc, err := cache.loadSchemaFromPath(ctx, tmpFile.Name(), 1*time.Hour, 30*time.Second, false) + if err == nil { + t.Fatal("a payload-directed load opened a local file") + } + assert.Contains(t, err.Error(), "only http and https are read") + assert.Nil(t, doc) + + // localSchema=true is an operator naming a path in the adapter's own + // config, which is the one case where opening a file is the intent. + doc, err = cache.loadSchemaFromPath(ctx, tmpFile.Name(), 1*time.Hour, 30*time.Second, true) assert.NoError(t, err) assert.NotNil(t, doc) assert.Equal(t, "3.1.0", doc.OpenAPI) } func TestLoadSchemaFromPath_CacheHit(t *testing.T) { + // A temp file is just the fixture here, so this loads in operator mode: + // a payload-directed load refuses local files by design. cache := newSchemaCache(10) ctx := context.Background() - + tmpFile, err := os.CreateTemp("", "test-schema-*.yaml") assert.NoError(t, err) defer os.Remove(tmpFile.Name()) - + schemaContent := `openapi: 3.1.0 info: title: Test Schema version: 1.0.0` - + tmpFile.Write([]byte(schemaContent)) tmpFile.Close() - - doc1, err := cache.loadSchemaFromPath(ctx, tmpFile.Name(), 1*time.Hour, 30*time.Second, false) + + doc1, err := cache.loadSchemaFromPath(ctx, tmpFile.Name(), 1*time.Hour, 30*time.Second, true) assert.NoError(t, err) - doc2, err := cache.loadSchemaFromPath(ctx, tmpFile.Name(), 1*time.Hour, 30*time.Second, false) + doc2, err := cache.loadSchemaFromPath(ctx, tmpFile.Name(), 1*time.Hour, 30*time.Second, true) assert.NoError(t, err) - + assert.Equal(t, doc1, doc2) } func TestLoadSchemaFromPath_InvalidPath(t *testing.T) { cache := newSchemaCache(10) ctx := context.Background() - + _, err := cache.loadSchemaFromPath(ctx, "/nonexistent/schema.yaml", 1*time.Hour, 30*time.Second, false) assert.Error(t, err) } func TestFindSchemaByType_DirectMatch(t *testing.T) { + // A temp file is just the fixture here, so this loads in operator mode: + // a payload-directed load refuses local files by design. cache := newSchemaCache(10) ctx := context.Background() - + tmpFile, err := os.CreateTemp("", "test-schema-*.yaml") assert.NoError(t, err) defer os.Remove(tmpFile.Name()) - + schemaContent := `openapi: 3.1.0 info: title: Test Schema @@ -538,11 +555,11 @@ components: properties: field1: type: string` - + tmpFile.Write([]byte(schemaContent)) tmpFile.Close() - - doc, err := cache.loadSchemaFromPath(ctx, tmpFile.Name(), 1*time.Hour, 30*time.Second, false) + + doc, err := cache.loadSchemaFromPath(ctx, tmpFile.Name(), 1*time.Hour, 30*time.Second, true) assert.NoError(t, err) schema, err := findSchemaByType(ctx, doc, "TestType") @@ -551,13 +568,15 @@ components: } func TestFindSchemaByType_NotFound(t *testing.T) { + // A temp file is just the fixture here, so this loads in operator mode: + // a payload-directed load refuses local files by design. cache := newSchemaCache(10) ctx := context.Background() - + tmpFile, err := os.CreateTemp("", "test-schema-*.yaml") assert.NoError(t, err) defer os.Remove(tmpFile.Name()) - + schemaContent := `openapi: 3.1.0 info: title: Test Schema @@ -566,11 +585,11 @@ components: schemas: TestType: type: object` - + tmpFile.Write([]byte(schemaContent)) tmpFile.Close() - - doc, err := cache.loadSchemaFromPath(ctx, tmpFile.Name(), 1*time.Hour, 30*time.Second, false) + + doc, err := cache.loadSchemaFromPath(ctx, tmpFile.Name(), 1*time.Hour, 30*time.Second, true) assert.NoError(t, err) _, err = findSchemaByType(ctx, doc, "NonExistentType") @@ -581,11 +600,7 @@ components: func TestValidateReferencedObject_Valid(t *testing.T) { cache := newSchemaCache(10) ctx := context.Background() - - tmpFile, err := os.CreateTemp("", "test-schema-*.yaml") - assert.NoError(t, err) - defer os.Remove(tmpFile.Name()) - + schemaContent := `openapi: 3.1.0 info: title: Test Schema @@ -603,33 +618,28 @@ components: type: string required: - field1` - - tmpFile.Write([]byte(schemaContent)) - tmpFile.Close() - + + ctxURL := serveTempSchema(t, schemaContent) + obj := referencedObject{ Path: "message.test", - Context: tmpFile.Name(), + Context: ctxURL, Type: "TestType", Data: map[string]interface{}{ - "@context": tmpFile.Name(), + "@context": ctxURL, "@type": "TestType", "field1": "value1", }, } - - err = cache.validateReferencedObject(ctx, obj, 1*time.Hour, 30*time.Second, nil, false) + + err := cache.validateReferencedObject(ctx, obj, 1*time.Hour, 30*time.Second, nil, false) assert.NoError(t, err) } func TestValidateReferencedObject_Invalid(t *testing.T) { cache := newSchemaCache(10) ctx := context.Background() - - tmpFile, err := os.CreateTemp("", "test-schema-*.yaml") - assert.NoError(t, err) - defer os.Remove(tmpFile.Name()) - + schemaContent := `openapi: 3.1.0 info: title: Test Schema @@ -647,21 +657,20 @@ components: type: string required: - field1` - - tmpFile.Write([]byte(schemaContent)) - tmpFile.Close() - + + ctxURL := serveTempSchema(t, schemaContent) + obj := referencedObject{ Path: "message.test", - Context: tmpFile.Name(), + Context: ctxURL, Type: "TestType", Data: map[string]interface{}{ - "@context": tmpFile.Name(), + "@context": ctxURL, "@type": "TestType", }, } - - err = cache.validateReferencedObject(ctx, obj, 1*time.Hour, 30*time.Second, nil, false) + + err := cache.validateReferencedObject(ctx, obj, 1*time.Hour, 30*time.Second, nil, false) assert.Error(t, err) schemaErrors := []model.Error{} @@ -698,10 +707,6 @@ func TestValidateReferencedObject_EntityTypeNotFound(t *testing.T) { cache := newSchemaCache(10) ctx := context.Background() - tmpFile, err := os.CreateTemp("", "test-schema-*.yaml") - assert.NoError(t, err) - defer os.Remove(tmpFile.Name()) - schemaContent := `openapi: 3.1.0 info: title: Test Schema @@ -711,20 +716,19 @@ components: TestType: type: object` - tmpFile.Write([]byte(schemaContent)) - tmpFile.Close() + ctxURL := serveTempSchema(t, schemaContent) obj := referencedObject{ Path: "message.test", - Context: tmpFile.Name(), + Context: ctxURL, Type: "NonExistentType", Data: map[string]interface{}{ - "@context": tmpFile.Name(), + "@context": ctxURL, "@type": "NonExistentType", }, } - err = cache.validateReferencedObject(ctx, obj, 1*time.Hour, 30*time.Second, nil, false) + err := cache.validateReferencedObject(ctx, obj, 1*time.Hour, 30*time.Second, nil, false) assert.Error(t, err) becknErr, ok := err.(*model.Error) @@ -831,7 +835,7 @@ components: - field1` tests := []struct { - name string + name string allowedDomains []string }{ {name: "file scheme allowed when no allowlist (nil)", allowedDomains: nil}, @@ -850,6 +854,8 @@ components: ctx := context.Background() // Use file:// scheme — would be rejected by scheme check if allowlist were set. + // It is still refused, one layer down: the reader takes http and + // https only. What an empty allowlist skips is the HOST check. obj := referencedObject{ Path: "message.test", Context: "file://" + tmpFile.Name(), @@ -858,7 +864,8 @@ components: } err = cache.validateReferencedObject(ctx, obj, 1*time.Hour, 30*time.Second, tt.allowedDomains, false) - // No domain or scheme error — allowlist check was skipped entirely. + // So: no domain error and no @context scheme error, which is what + // an empty allowlist means. Not "anything is readable". if err != nil { assert.NotContains(t, err.Error(), "domain not allowed") assert.NotContains(t, err.Error(), "invalid scheme in @context") @@ -875,14 +882,14 @@ func TestValidateExtendedSchemas_NoObjects(t *testing.T) { }, schemaCache: newSchemaCache(10), } - + ctx := context.Background() body := map[string]interface{}{ "message": map[string]interface{}{ "field": "value", }, } - + err := v.validateExtendedSchemas(ctx, body) assert.NoError(t, err) } @@ -894,12 +901,12 @@ func TestValidateExtendedSchemas_MissingMessage(t *testing.T) { }, schemaCache: newSchemaCache(10), } - + ctx := context.Background() body := map[string]interface{}{ "context": map[string]interface{}{}, } - + err := v.validateExtendedSchemas(ctx, body) assert.Error(t, err) assert.Contains(t, err.Error(), "missing 'message' field") @@ -1055,9 +1062,9 @@ func TestIsSchemaVersionSegment(t *testing.T) { func TestExtractRelativeSchemaPath(t *testing.T) { tests := []struct { - name string + name string rawURL string - want string + want string }{ { name: "URL with /schema/ marker and version", @@ -1275,11 +1282,7 @@ func TestValidateReferencedObject_LocalMissFallsBackToContext(t *testing.T) { cache := newSchemaCache(10) ctx := context.Background() - tmpFile, err := os.CreateTemp("", "test-schema-*.yaml") - assert.NoError(t, err) - defer os.Remove(tmpFile.Name()) - - tmpFile.Write([]byte(`openapi: 3.1.0 + ctxURL := serveTempSchema(t, `openapi: 3.1.0 info: title: Test Schema version: 1.0.0 @@ -1289,22 +1292,21 @@ components: type: object properties: field1: - type: string`)) - tmpFile.Close() + type: string`) obj := referencedObject{ Path: "message.test", - Context: tmpFile.Name(), + Context: ctxURL, Type: "TestType", Data: map[string]interface{}{ - "@context": tmpFile.Name(), + "@context": ctxURL, "@type": "TestType", "field1": "value1", }, } - // rawSchemas empty, localSchema=true — local miss, falls back to @context file path - err = cache.validateReferencedObject(ctx, obj, 1*time.Hour, 30*time.Second, nil, true) + // rawSchemas empty, localSchema=true — local miss, falls back to fetching the @context + err := cache.validateReferencedObject(ctx, obj, 1*time.Hour, 30*time.Second, nil, true) assert.NoError(t, err) } @@ -1409,15 +1411,12 @@ components: type: string const: openagrinet:WeatherObservation` -func writeTempSchema(t *testing.T, content string) string { +// serveTempSchema serves content over http and returns a URL usable as an +// @context. Served rather than written to disk because a payload-directed load +// reads http and https only -- and because fetching is what production does. +func serveTempSchema(t *testing.T, content string) string { t.Helper() - f, err := os.CreateTemp("", "test-schema-*.yaml") - assert.NoError(t, err) - t.Cleanup(func() { os.Remove(f.Name()) }) - _, err = f.Write([]byte(content)) - assert.NoError(t, err) - assert.NoError(t, f.Close()) - return f.Name() + return serveSchema(t, content).URL + "/context.jsonld" } // A pack that requires @type must receive it. This is the case that could not @@ -1426,7 +1425,7 @@ func writeTempSchema(t *testing.T, content string) string { // "@type is required". func TestValidateReferencedObject_PackStyleKeepsAtType(t *testing.T) { cache := newSchemaCache(10) - path := writeTempSchema(t, packStyleSchema) + path := serveTempSchema(t, packStyleSchema) obj := referencedObject{ Path: "message.catalogs[0].resources[0].resourceAttributes", @@ -1448,7 +1447,7 @@ func TestValidateReferencedObject_PackStyleKeepsAtType(t *testing.T) { // stripping it silently skipped. func TestValidateReferencedObject_PackStyleWrongAtTypeRejected(t *testing.T) { cache := newSchemaCache(10) - path := writeTempSchema(t, packStyleSchema) + path := serveTempSchema(t, packStyleSchema) obj := referencedObject{ Path: "message.catalogs[0].resources[0].resourceAttributes", @@ -1570,3 +1569,165 @@ func TestSchemaDeclaresProperty(t *testing.T) { }) } } + +// A payload chooses the @context, so it chooses every document the loader then +// reads to resolve that document's $refs. The allowlist is consulted once, on +// the entry URL; these tests cover what happens after it. + +const entrySchemaRefTemplate = `openapi: 3.1.0 +info: + title: entry + version: "1" +paths: {} +components: + schemas: + TestType: + type: object + properties: + field1: + $ref: "REF_TARGET#/components/schemas/Borrowed" +` + +const borrowedSchema = `openapi: 3.1.0 +info: + title: borrowed + version: "1" +paths: {} +components: + schemas: + Borrowed: + type: string +` + +// serveSchema returns an https-less test server answering every path with body. +func serveSchema(t *testing.T, body string) *httptest.Server { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/yaml") + _, _ = w.Write([]byte(body)) + })) + t.Cleanup(srv.Close) + return srv +} + +func TestValidateReferencedObject_RefusesARefThatWouldReadTheDisk(t *testing.T) { + // A real file, so a successful read would be indistinguishable from a + // legitimate schema and the test could not tell the two apart. + onDisk := filepath.Join(t.TempDir(), "borrowed.yaml") + if err := os.WriteFile(onDisk, []byte(borrowedSchema), 0o600); err != nil { + t.Fatalf("failed to write the file under test: %v", err) + } + + for _, tt := range []struct { + name string + ref string + }{ + {name: "file scheme", ref: "file://" + onDisk}, + {name: "bare path, which parses with no scheme at all", ref: onDisk}, + } { + t.Run(tt.name, func(t *testing.T) { + entry := serveSchema(t, strings.Replace(entrySchemaRefTemplate, "REF_TARGET", tt.ref, 1)) + host, err := url.Parse(entry.URL) + if err != nil { + t.Fatalf("failed to parse the test server URL: %v", err) + } + + cache := newSchemaCache(10) + obj := referencedObject{ + Path: "message.test", + Context: entry.URL + "/context.jsonld", + Type: "TestType", + Data: map[string]interface{}{"field1": "value1"}, + } + + err = cache.validateReferencedObject(context.Background(), obj, + 1*time.Hour, 30*time.Second, []string{host.Host}, false) + + // The entry document is allowlisted and https, so nothing before + // the $ref refuses this. Only the reader can. + if err == nil { + t.Fatal("the $ref was read, so a payload can name any file on disk") + } + assert.Contains(t, err.Error(), "refusing to read schema from") + }) + } +} + +// The packs pull 15 documents across 3 hosts -- the one the allowlist names +// plus two external spec hosts the packs $ref into -- so a $ref to a host +// outside the allowlist is the normal case, not the attack. This pins that: +// applying isAllowedDomain to $refs as well would need all three hosts named +// in the allowlist first, and would otherwise stop every pack loading. +// Deliberate, not missed. +func TestValidateReferencedObject_AllowsARefToAHostOutsideTheAllowlist(t *testing.T) { + borrowed := serveSchema(t, borrowedSchema) + entry := serveSchema(t, strings.Replace(entrySchemaRefTemplate, "REF_TARGET", borrowed.URL+"/borrowed.yaml", 1)) + + entryHost, err := url.Parse(entry.URL) + if err != nil { + t.Fatalf("failed to parse the test server URL: %v", err) + } + borrowedHost, err := url.Parse(borrowed.URL) + if err != nil { + t.Fatalf("failed to parse the test server URL: %v", err) + } + if entryHost.Port() == borrowedHost.Port() { + t.Fatal("the two servers must differ, or this proves nothing") + } + + cache := newSchemaCache(10) + obj := referencedObject{ + Path: "message.test", + Context: entry.URL + "/context.jsonld", + Type: "TestType", + Data: map[string]interface{}{"field1": "value1"}, + } + + // Only the entry host is allowlisted; the $ref host is not. + if err := cache.validateReferencedObject(context.Background(), obj, + 1*time.Hour, 30*time.Second, []string{entryHost.Host}, false); err != nil { + t.Fatalf("a cross-host $ref must still resolve, or no pack can load: %v", err) + } +} + +func TestPayloadDirectedReader(t *testing.T) { + onDisk := filepath.Join(t.TempDir(), "schema.yaml") + if err := os.WriteFile(onDisk, []byte(borrowedSchema), 0o600); err != nil { + t.Fatalf("failed to write the file under test: %v", err) + } + + for _, tt := range []struct { + name string + raw string + refused bool + }{ + {name: "file scheme", raw: "file://" + onDisk, refused: true}, + {name: "bare path", raw: onDisk, refused: true}, + {name: "a scheme nobody serves schemas over", raw: "gopher://example.test/schema.yaml", refused: true}, + {name: "http is read", raw: "", refused: false}, + } { + t.Run(tt.name, func(t *testing.T) { + raw := tt.raw + if raw == "" { + raw = serveSchema(t, borrowedSchema).URL + "/schema.yaml" + } + u, err := url.Parse(raw) + if err != nil { + t.Fatalf("failed to parse %q: %v", raw, err) + } + + data, err := payloadDirectedReader(openapi3.NewLoader(), u) + if tt.refused { + if err == nil { + t.Fatalf("%q was read, and must not have been", raw) + } + assert.Contains(t, err.Error(), "only http and https are read") + // The point is that nothing was read, not merely that it errored. + assert.Empty(t, data) + return + } + assert.NoError(t, err) + assert.Contains(t, string(data), "Borrowed") + }) + } +} From 9a1594f32c1c45f99eeda9c7daf78332cabbfd28 Mon Sep 17 00:00:00 2001 From: ameersohel45 Date: Tue, 8 Sep 2026 00:59:23 +0530 Subject: [PATCH 08/10] fix(schemav2validator): read the list form of @type, and reject what it cannot read [#16] findReferencedObjects required a string for both JSON-LD keys, so an object carrying the list form matched neither assertion, was never appended, and its resourceAttributes went to no schema at all -- while the extended layer reported a pass over an object it had not looked at. The list form is not exotic. The packs declare @type as a oneOf whose second branch is a list carrying the canonical OAN type alongside provider-defined ones, so a conforming payload could use it and be skipped. Which entry names the capability is not fixed either, so the document now decides: the first @type it declares a schema for wins, rather than the payload's ordering. @context takes the list form too, using the first string in it, because only a URL locates a schema and an inline object names no document to fetch. Skipping is replaced by rejection wherever the object claims a type this validator cannot read. @type ABSENT is left alone and still passes over: an object with a context and no type makes no claim about which schema applies, and there is nothing to validate it against. Also replaces the @type const test. It built obj.Type and Data["@type"] disagreeing, which the real path cannot produce -- both are read off one map -- so it demonstrated nothing about a payload. Tests now run through findReferencedObjects, and the const does have a payload-level case: the list branch forbids a second openagrinet: type, which is caught only because @type is kept in the data rather than stripped. --- .../schemav2validator/extended_schema.go | 149 +++++++++-- .../schemav2validator/extended_schema_test.go | 248 ++++++++++++++++-- 2 files changed, 365 insertions(+), 32 deletions(-) diff --git a/pkg/plugin/implementation/schemav2validator/extended_schema.go b/pkg/plugin/implementation/schemav2validator/extended_schema.go index 2fdcb614..8c71a5ea 100644 --- a/pkg/plugin/implementation/schemav2validator/extended_schema.go +++ b/pkg/plugin/implementation/schemav2validator/extended_schema.go @@ -79,7 +79,29 @@ type referencedObject struct { Path string Context string Type string - Data map[string]interface{} + // Types is every @type the object carries, in payload order. JSON-LD + // permits the array form and the packs allow it explicitly, so which + // entry names the capability is not known until a document is consulted. + Types []string + Data map[string]interface{} + // Unusable is set when the object claims a domain type but carries it in + // a shape this validator cannot resolve. It is a coded error rather than + // a bool because the object is then rejected, not skipped: a layer whose + // purpose is to turn a missing attribute into a rejection must not answer + // "valid" for an object it never looked at. + Unusable error +} + +// candidateTypes returns the @type values to try, tolerating an object built +// with Type alone -- which every caller outside findReferencedObjects does. +func (o referencedObject) candidateTypes() []string { + if len(o.Types) > 0 { + return o.Types + } + if o.Type == "" { + return nil + } + return []string{o.Type} } // schemaCache caches loaded domain schemas with LRU eviction. @@ -445,16 +467,29 @@ func findReferencedObjects(data interface{}, path string) []referencedObject { switch v := data.(type) { case map[string]interface{}: - // Check for @context and @type - if contextVal, hasContext := v["@context"].(string); hasContext { - if typeVal, hasType := v["@type"].(string); hasType { - results = append(results, referencedObject{ - Path: path, - Context: contextVal, - Type: typeVal, - Data: v, - }) + // @type ABSENT is not the same as @type unreadable. An object with a + // context and no type makes no claim about which schema applies, and + // there is nothing to validate it against, so it is passed over as + // before. An object that does claim a type is validated or rejected. + rawContext, hasContext := v["@context"] + rawType, hasType := v["@type"] + if hasContext && hasType { + obj := referencedObject{Path: path, Data: v} + contextVal, contextOK := jsonLDLocation(rawContext) + types, typesOK := jsonLDTypes(rawType) + switch { + case !contextOK: + obj.Unusable = model.NewCodedError("SCH_INVALID_JSONLD_CONTEXT", + "@context is not a URL this validator can resolve a schema from") + case !typesOK: + obj.Unusable = model.NewCodedError("SCH_INVALID_ENTITY_TYPE", + "@type is present but is not a type name or a list of them") + default: + obj.Context = contextVal + obj.Type = types[0] + obj.Types = types } + results = append(results, obj) } // Recurse into nested objects @@ -483,6 +518,70 @@ func transformContextToSchemaURL(contextURL string) string { return strings.Replace(contextURL, "context.jsonld", "attributes.yaml", 1) } +// jsonLDLocation returns the @context entry a schema can be located from. +// +// JSON-LD allows a string, an array mixing strings and inline objects, or a +// single inline object. Only a URL locates a schema, so the first string is +// taken and an inline object yields nothing -- there is no document to fetch. +func jsonLDLocation(raw interface{}) (string, bool) { + switch v := raw.(type) { + case string: + if v != "" { + return v, true + } + case []interface{}: + for _, entry := range v { + if s, ok := entry.(string); ok && s != "" { + return s, true + } + } + } + return "", false +} + +// jsonLDTypes returns every @type the object carries, in payload order. +// +// The array form is not exotic: the packs declare @type as a oneOf whose +// second branch is an array containing the canonical OAN type plus +// provider-defined ones. Reading only the string form left those objects +// matching nothing, so they were dropped before validation and the layer +// reported a pass over an object it had not looked at. +func jsonLDTypes(raw interface{}) ([]string, bool) { + switch v := raw.(type) { + case string: + if v != "" { + return []string{v}, true + } + case []interface{}: + types := make([]string, 0, len(v)) + for _, entry := range v { + if s, ok := entry.(string); ok && s != "" { + types = append(types, s) + } + } + if len(types) > 0 { + return types, true + } + } + return nil, false +} + +// findSchemaForAnyType resolves the first @type the document declares a schema +// for, and returns which one matched. With the array form the capability type +// sits among provider-defined ones and its position is not fixed, so the +// document decides rather than the payload's ordering. +func findSchemaForAnyType(ctx context.Context, doc *openapi3.T, types []string) (*openapi3.SchemaRef, string, error) { + var lastErr error + for _, typeName := range types { + schema, err := findSchemaByType(ctx, doc, typeName) + if err == nil { + return schema, typeName, nil + } + lastErr = err + } + return nil, "", lastErr +} + // findSchemaByType finds a schema in the document by @type value. func findSchemaByType(ctx context.Context, doc *openapi3.T, typeName string) (*openapi3.SchemaRef, error) { if doc.Components == nil || doc.Components.Schemas == nil { @@ -600,18 +699,31 @@ func (c *schemaCache) validateReferencedObject( allowedDomains []string, localSchema bool, ) error { + // An object that claims a domain type in a shape we cannot resolve is + // rejected here rather than dropped in findReferencedObjects. Dropping it + // meant the extended layer reported a pass over an object it never + // validated, which is the one outcome this layer exists to prevent. + if obj.Unusable != nil { + log.Warnf(ctx, "refusing an object at %s that carries @context in an unusable shape: %v", obj.Path, obj.Unusable) + return obj.Unusable + } + var doc *openapi3.T if localSchema { - typeName := obj.Type - if idx := strings.LastIndex(typeName, ":"); idx >= 0 { - typeName = typeName[idx+1:] - } - if typeName != "" && !strings.ContainsAny(typeName, "/\\") { + for _, candidate := range obj.candidateTypes() { + typeName := candidate + if idx := strings.LastIndex(typeName, ":"); idx >= 0 { + typeName = typeName[idx+1:] + } + if typeName == "" || strings.ContainsAny(typeName, "/\\") { + continue + } if localDoc, localErr := c.loadSchemaFromPath(ctx, typeName+"/attributes.yaml", ttl, timeout, localSchema); localErr != nil { - log.Debugf(ctx, "local @type lookup failed for %s: %v", obj.Type, localErr) + log.Debugf(ctx, "local @type lookup failed for %s: %v", candidate, localErr) } else { doc = localDoc + break } } } @@ -640,7 +752,10 @@ func (c *schemaCache) validateReferencedObject( } // Find schema by @type - schema, err := findSchemaByType(ctx, doc, obj.Type) + schema, matched, err := findSchemaForAnyType(ctx, doc, obj.candidateTypes()) + if err == nil && matched != obj.Type { + log.Debugf(ctx, "resolved @type %s from the array at %s", matched, obj.Path) + } if err != nil { log.Errorf(ctx, err, "Schema not found for @type: %s at path: %s", obj.Type, obj.Path) return model.NewCodedErrorWithCause("SCH_INVALID_ENTITY_TYPE", err.Error(), obj.Path, err) diff --git a/pkg/plugin/implementation/schemav2validator/extended_schema_test.go b/pkg/plugin/implementation/schemav2validator/extended_schema_test.go index 39beebff..8a6f5fae 100644 --- a/pkg/plugin/implementation/schemav2validator/extended_schema_test.go +++ b/pkg/plugin/implementation/schemav2validator/extended_schema_test.go @@ -1442,25 +1442,243 @@ func TestValidateReferencedObject_PackStyleKeepsAtType(t *testing.T) { assert.NoError(t, err) } -// The same pack must still reject a payload whose @type is not the one the -// capability declares -- keeping the key means its const is now checked, which -// stripping it silently skipped. -func TestValidateReferencedObject_PackStyleWrongAtTypeRejected(t *testing.T) { - cache := newSchemaCache(10) - path := serveTempSchema(t, packStyleSchema) +// packStyleTypeListSchema mirrors how the packs really declare @type: a oneOf +// whose first branch is the canonical string and whose second is a list +// carrying that type alongside provider-defined ones, which must not take the +// openagrinet: prefix. +const packStyleTypeListSchema = `openapi: 3.1.0 +info: + title: Pack Style With Type List + version: 1.0.0 +components: + schemas: + WeatherObservation: + type: object + x-jsonld: + "@context": https://schemas.example.org/schema/WeatherObservation/v0.1/context.jsonld + "@type": openagrinet:WeatherObservation + allOf: + - type: object + required: + - informationMode + properties: + informationMode: + type: string + enum: [OnDemand, Direct] + - type: object + required: + - "@type" + properties: + "@type": + oneOf: + - type: string + const: openagrinet:WeatherObservation + - type: array + minItems: 2 + uniqueItems: true + contains: + const: openagrinet:WeatherObservation + items: + oneOf: + - const: openagrinet:WeatherObservation + - type: string + minLength: 1 + not: + pattern: "^openagrinet:"` + +// resourceBody wraps resourceAttributes the way a payload carries them, so +// discovery runs over the same shape production sees. +func resourceBody(ctxURL string, atType interface{}, informationMode string) map[string]interface{} { + attrs := map[string]interface{}{"@context": ctxURL, "@type": atType} + if informationMode != "" { + attrs["informationMode"] = informationMode + } + return map[string]interface{}{ + "message": map[string]interface{}{ + "catalogs": []interface{}{ + map[string]interface{}{"resources": []interface{}{ + map[string]interface{}{"resourceAttributes": attrs}, + }}, + }, + }, + } +} - obj := referencedObject{ - Path: "message.catalogs[0].resources[0].resourceAttributes", - Context: path, - Type: "openagrinet:WeatherObservation", - Data: map[string]interface{}{ - "@type": "openagrinet:MandiPrice", - "informationMode": "OnDemand", +// theObjectIn runs the production discovery over a body and returns the single +// domain object in it. Tests go through this rather than building a +// referencedObject by hand: Context, Type and Data all come off one map there, +// so a hand-built object can assert a state the real path cannot produce. +func theObjectIn(t *testing.T, body map[string]interface{}) referencedObject { + t.Helper() + objects := findReferencedObjects(body["message"], "message") + if len(objects) != 1 { + t.Fatalf("expected exactly one domain object from discovery, got %d", len(objects)) + } + return objects[0] +} + +// The pack allows @type to be a list, and reading only the string form meant +// such an object matched nothing, was dropped before validation, and the layer +// reported a pass over a payload it had not looked at. +func TestValidateReferencedObject_AcceptsAndChecksATypeList(t *testing.T) { + ctxURL := serveTempSchema(t, packStyleTypeListSchema) + const canonical = "openagrinet:WeatherObservation" + + for _, tt := range []struct { + name string + atType interface{} + wantErr bool + wantErrHas string + }{ + { + name: "the canonical type alone, as a string", + atType: canonical, + }, + { + name: "the canonical type beside a provider type", + atType: []interface{}{canonical, "vendor:GriddedForecast"}, + }, + { + name: "provider type first -- the document decides which entry names the capability", + atType: []interface{}{"vendor:GriddedForecast", canonical}, + }, + { + // A real payload-level rejection, and one that only bites because + // @type is kept in the data rather than stripped: the list branch + // forbids a second openagrinet: type. + name: "a second openagrinet type, which the pack forbids", + atType: []interface{}{canonical, "openagrinet:MandiPrice"}, + wantErr: true, }, + { + // The string branch does not match a list and the list branch + // requires two entries, so neither is satisfied. + name: "a single-entry list, which satisfies neither branch", + atType: []interface{}{canonical}, + wantErr: true, + }, + { + name: "a list naming no type the document declares", + atType: []interface{}{"vendor:One", "vendor:Two"}, + wantErr: true, + wantErrHas: "no schema found", + }, + { + name: "@type present but not a type name", + atType: 42, + wantErr: true, + wantErrHas: "not a type name", + }, + { + name: "@type an empty list", + atType: []interface{}{}, + wantErr: true, + wantErrHas: "not a type name", + }, + } { + t.Run(tt.name, func(t *testing.T) { + obj := theObjectIn(t, resourceBody(ctxURL, tt.atType, "OnDemand")) + err := newSchemaCache(10).validateReferencedObject( + context.Background(), obj, 1*time.Hour, 30*time.Second, nil, false) + + if !tt.wantErr { + assert.NoError(t, err) + return + } + if err == nil { + t.Fatal("expected a rejection; a skipped object is reported as valid") + } + if tt.wantErrHas != "" { + assert.Contains(t, err.Error(), tt.wantErrHas) + } + }) } +} - err := cache.validateReferencedObject(context.Background(), obj, 1*time.Hour, 30*time.Second, nil, false) - assert.Error(t, err) +// An object claiming a type this validator cannot read must be rejected, not +// passed over. Skipping is what let unvalidated resourceAttributes through. +func TestFindReferencedObjects_TypeShapes(t *testing.T) { + const ctxURL = "https://schemas.example.org/schema/WeatherObservation/v0.1/context.jsonld" + + for _, tt := range []struct { + name string + attrs map[string]interface{} + wantFound bool + wantTypes []string + wantCode string + }{ + { + name: "string @type", + attrs: map[string]interface{}{"@context": ctxURL, "@type": "openagrinet:WeatherObservation"}, + wantFound: true, + wantTypes: []string{"openagrinet:WeatherObservation"}, + }, + { + name: "list @type keeps every entry, in payload order", + attrs: map[string]interface{}{"@context": ctxURL, "@type": []interface{}{"a", "b"}}, + wantFound: true, + wantTypes: []string{"a", "b"}, + }, + { + name: "list @context takes the first string, since only a URL locates a schema", + attrs: map[string]interface{}{"@context": []interface{}{ctxURL, map[string]interface{}{"inline": "term"}}, "@type": "T"}, + wantFound: true, + wantTypes: []string{"T"}, + }, + { + name: "inline-object @context names no document to fetch", + attrs: map[string]interface{}{"@context": map[string]interface{}{"inline": "term"}, "@type": "T"}, + wantFound: true, + wantCode: "SCH_INVALID_JSONLD_CONTEXT", + }, + { + name: "@type a number", + attrs: map[string]interface{}{"@context": ctxURL, "@type": 42}, + wantFound: true, + wantCode: "SCH_INVALID_ENTITY_TYPE", + }, + { + // No claim about which schema applies, so there is nothing to + // validate against. Passed over, as before. + name: "@context with no @type at all", + attrs: map[string]interface{}{"@context": ctxURL, "field": "value"}, + wantFound: false, + }, + } { + t.Run(tt.name, func(t *testing.T) { + objects := findReferencedObjects(map[string]interface{}{"resourceAttributes": tt.attrs}, "message") + if !tt.wantFound { + assert.Empty(t, objects) + return + } + if len(objects) != 1 { + t.Fatalf("expected one object, got %d", len(objects)) + } + obj := objects[0] + + if tt.wantCode != "" { + if obj.Unusable == nil { + t.Fatal("expected the object to be marked unusable, so it is rejected rather than skipped") + } + becknErr, ok := obj.Unusable.(*model.Error) + if !ok { + t.Fatalf("Unusable = %T, want *model.Error", obj.Unusable) + } + assert.Equal(t, tt.wantCode, becknErr.Code) + + // and it must actually reject when validated + err := newSchemaCache(10).validateReferencedObject( + context.Background(), obj, 1*time.Hour, 30*time.Second, nil, false) + assert.Error(t, err) + return + } + + assert.Nil(t, obj.Unusable) + assert.Equal(t, tt.wantTypes, obj.Types) + assert.Equal(t, tt.wantTypes[0], obj.Type) + assert.Equal(t, ctxURL, obj.Context) + }) + } } func TestStripUnaccountedJSONLDKeys(t *testing.T) { From 5132832bf89eea3ab11a38887898d0b84b8df40e Mon Sep 17 00:00:00 2001 From: ameersohel45 Date: Tue, 8 Sep 2026 17:55:22 +0530 Subject: [PATCH 09/10] fix(schemav2validator): enforce the host allowlist on every schema read [#16] The allowlist guarded the entry @context only. The document that came back was then trusted to name anything: its $refs resolved through the loader with no host check, so a payload could name an attacker's document on the allowed host and have this process fetch whatever that document pointed at -- an internal service, a cloud metadata endpoint. Server-side request forgery, driven by an unauthenticated field. The reach was the argument for fixing it rather than documenting it: loading one capability pack pulls 13-16 documents, of which exactly one is the entry. The refs were never the corner case, they were the traffic. The same allowlist is now checked on every read. Two refusals with different scopes, because they answer different questions: scheme http/https only, and only on the payload-directed path. An operator who set extendedSchema_localSchemaPath is asking for files to be read, so the refusal must not apply to them -- installing it on both branches broke exactly that, which TestLoadSchemaFromPath_LocalFile caught. host on BOTH paths. localSchema falls back to the network for a ref it does not hold locally, so it had the same exposure by a longer route. THE ALLOWLIST HAD TO GROW, and this is a code+config pair that cannot land half-applied. Measured, not assumed: every pack -- WeatherObservation, MandiPrice, KnowledgeResource -- touches raw.githubusercontent.com, schema.beckn.io and schema.nfh.global. With the previous single host and refs checked, the real packs fail with SCH_SCHEMA_ADAPTATION_FAILED on every payload; with the three named, they load and validate. Both directions are verified against the published packs. The test that pinned the old behaviour is inverted rather than deleted: it now asserts a cross-host $ref is refused, and a second test asserts the chain loads once both hosts are named -- which is the case the packs need. What this does NOT fix: raw.githubusercontent.com is world-writable, so the allowlist still trusts every GitHub account for schema content. Narrowing to a path prefix, or mirroring the packs on a host we control, is the real fix and is not a one-line change. Said so in the config. --- config/provider-adapter.yaml | 26 +++++- .../schemav2validator/extended_schema.go | 85 ++++++++++++------- .../schemav2validator/extended_schema_test.go | 84 +++++++++++++----- 3 files changed, 141 insertions(+), 54 deletions(-) diff --git a/config/provider-adapter.yaml b/config/provider-adapter.yaml index e892811b..b2329ab0 100644 --- a/config/provider-adapter.yaml +++ b/config/provider-adapter.yaml @@ -207,8 +207,30 @@ modules: # the first payload after a restart pays for it. A fetch that # FAILS rejects the payload -- it does not skip validation, which # is the right way round, but it does mean this adapter needs - # egress to the host allowed below. - extendedSchema_allowedDomains: "raw.githubusercontent.com" + # egress to every host allowed below. + # + # THIS IS THE WHOLE TRUST BOUNDARY, and it is checked on every + # read: the entry @context and every $ref under it. That matters + # because the document a payload names is NOT trusted -- it comes + # from a URL the payload chose, on a host anyone can publish to -- + # so without the check its $refs could send this process at an + # internal service or a cloud metadata endpoint. + # + # ALL THREE HOSTS ARE REQUIRED. Loading one capability pack pulls + # 13-16 documents across exactly these three (measured, not + # assumed): the pack itself and AgricultureResource from the raw + # CDN, then Descriptor/GeoJSONGeometry/Location/Address from + # schema.beckn.io, which in turn $ref schema.nfh.global. Remove + # any one and no pack loads at all -- the failure is + # SCH_SCHEMA_ADAPTATION_FAILED on every payload, not a partial + # validation. + # + # Keep it as tight as the packs allow. raw.githubusercontent.com + # is world-writable, so this trusts every GitHub account for + # schema content; narrowing it to a path prefix, or mirroring the + # packs on a host we control, is the real fix and is not a + # one-line change. + extendedSchema_allowedDomains: "raw.githubusercontent.com,schema.beckn.io,schema.nfh.global" extendedSchema_cacheTTL: "86400" # 24h extendedSchema_maxCacheSize: "100" diff --git a/pkg/plugin/implementation/schemav2validator/extended_schema.go b/pkg/plugin/implementation/schemav2validator/extended_schema.go index 8c71a5ea..1c3d7169 100644 --- a/pkg/plugin/implementation/schemav2validator/extended_schema.go +++ b/pkg/plugin/implementation/schemav2validator/extended_schema.go @@ -335,7 +335,7 @@ func (c *schemaCache) cleanupExpired() int { return len(expired) } -func (c *schemaCache) loadSchemaFromPath(ctx context.Context, schemaPath string, ttl, timeout time.Duration, localSchema bool) (*openapi3.T, error) { +func (c *schemaCache) loadSchemaFromPath(ctx context.Context, schemaPath string, ttl, timeout time.Duration, allowedDomains []string, localSchema bool) (*openapi3.T, error) { urlHash := hashURL(schemaPath) u, parseErr := url.Parse(schemaPath) @@ -345,15 +345,18 @@ func (c *schemaCache) loadSchemaFromPath(ctx context.Context, schemaPath string, loader := newFreshLoader() loader.Context = ctx - if !localSchema { - // The schema location on this path is derived from a payload's - // @context, so every read it causes is network-directed. Installed - // here rather than at the one @context check because that check runs - // once, on the entry document: the $refs inside whatever comes back - // are resolved by the loader and meet no check at all. One pack pulls - // 15 documents across 3 hosts, so this is the majority of the reads. - loader.ReadFromURIFunc = payloadDirectedReader - } + // Installed on BOTH branches, with the local-file allowance following + // localSchema. The check at the one @context runs once, on the entry + // document; the $refs inside whatever comes back are resolved by the + // loader, and a pack pulls 13-16 documents, so the refs are the great + // majority of the reads. + // + // localSchema is included because its rawSchemas path falls back to the + // NETWORK for a ref it does not hold -- so a local document could reach an + // arbitrary host, the same exposure by a longer route. What it keeps is + // the file read itself, which in that mode is the operator's stated + // intent rather than something a payload asked for. + loader.ReadFromURIFunc = payloadDirectedReader(allowedDomains, localSchema) var doc *openapi3.T var err error @@ -440,25 +443,49 @@ func (c *schemaCache) loadSchemaFromPath(ctx context.Context, schemaPath string, return doc, nil } -// payloadDirectedReader reads a schema document for a location that a payload -// chose, refusing any scheme but http and https. +// payloadDirectedReader returns a reader for schema documents whose location a +// payload chose, enforcing the allowlist on EVERY read -- the entry document +// and every $ref under it. +// +// Two separate refusals, for two separate reasons. +// +// SCHEME: freshReadFromURI falls through to os.ReadFile for every scheme but +// http and https, so a $ref of "file:///etc/passwd" -- or a bare path, which +// parses with no scheme at all -- is an instruction from the network to read +// this container's disk and parse it as a schema. The base spec loader keeps +// that fallthrough deliberately: its location is operator-configured, where a +// local file is the point. Here it never is. +// +// HOST: the entry @context is allowlisted, but the document it returns is not +// trusted -- it came from a payload-named URL on a host anyone can publish to. +// Its $refs used to reach any http host at all, so a payload could name an +// attacker's document and have this process fetch whatever that document +// pointed at: an internal service, a cloud metadata endpoint. Checking the +// same allowlist on every read closes that, and makes the allowlist mean what +// it says -- the hosts this deployment will read schemas from, not the hosts +// it will read the FIRST schema from. // -// freshReadFromURI falls through to os.ReadFile for every other scheme, so -// without this a $ref of "file:///etc/passwd" -- or a bare path, which parses -// with no scheme at all -- is an instruction from the network to read this -// container's disk and parse it as a schema. The base spec loader keeps that -// fallthrough deliberately: its location is operator-configured, where a local -// file is the point. Here it never is. +// This is why the allowlist cannot be a single host: loading one capability +// pack touches raw.githubusercontent.com, schema.beckn.io and +// schema.nfh.global (13-16 reads, measured), so all three have to be named or +// no pack loads at all. An empty allowlist still means "unset, do not check", +// as it does at the @context. // -// This does NOT restrict which hosts may be reached; isAllowedDomain still -// guards only the entry @context. Enforcing the allowlist here as well is the -// right shape, but the packs $ref two external spec hosts, so it needs those -// named in the allowlist or no pack loads at all. -func payloadDirectedReader(loader *openapi3.Loader, u *url.URL) ([]byte, error) { - if u.Scheme != "http" && u.Scheme != "https" { - return nil, fmt.Errorf("refusing to read schema from %q: only http and https are read for a location a payload chose", u.String()) - } - return freshReadFromURI(loader, u) +// allowLocal follows localSchema: an operator who configured +// extendedSchema_localSchemaPath is asking for files to be read, so the scheme +// refusal does not apply to them. The HOST check still does, because that +// mode falls back to the network for a ref it does not hold locally. +func payloadDirectedReader(allowedDomains []string, allowLocal bool) func(*openapi3.Loader, *url.URL) ([]byte, error) { + return func(loader *openapi3.Loader, u *url.URL) ([]byte, error) { + remote := u.Scheme == "http" || u.Scheme == "https" + if !remote && !allowLocal { + return nil, fmt.Errorf("refusing to read schema from %q: only http and https are read for a location a payload chose", u.String()) + } + if remote && len(allowedDomains) > 0 && !isAllowedDomain(u, allowedDomains) { + return nil, fmt.Errorf("refusing to read schema from %q: host is not in extendedSchema_allowedDomains", u.String()) + } + return freshReadFromURI(loader, u) + } } // findReferencedObjects recursively finds domain-specific objects with @context. @@ -719,7 +746,7 @@ func (c *schemaCache) validateReferencedObject( if typeName == "" || strings.ContainsAny(typeName, "/\\") { continue } - if localDoc, localErr := c.loadSchemaFromPath(ctx, typeName+"/attributes.yaml", ttl, timeout, localSchema); localErr != nil { + if localDoc, localErr := c.loadSchemaFromPath(ctx, typeName+"/attributes.yaml", ttl, timeout, allowedDomains, localSchema); localErr != nil { log.Debugf(ctx, "local @type lookup failed for %s: %v", candidate, localErr) } else { doc = localDoc @@ -745,7 +772,7 @@ func (c *schemaCache) validateReferencedObject( schemaPath := transformContextToSchemaURL(obj.Context) log.Debugf(ctx, "Transformed %s -> %s (localSchema=%v)", obj.Context, schemaPath, localSchema) var err error - doc, err = c.loadSchemaFromPath(ctx, schemaPath, ttl, timeout, false) + doc, err = c.loadSchemaFromPath(ctx, schemaPath, ttl, timeout, allowedDomains, false) if err != nil { return model.NewCodedErrorWithCause("SCH_SCHEMA_ADAPTATION_FAILED", err.Error(), obj.Path, err) } diff --git a/pkg/plugin/implementation/schemav2validator/extended_schema_test.go b/pkg/plugin/implementation/schemav2validator/extended_schema_test.go index 8a6f5fae..9facca44 100644 --- a/pkg/plugin/implementation/schemav2validator/extended_schema_test.go +++ b/pkg/plugin/implementation/schemav2validator/extended_schema_test.go @@ -484,7 +484,7 @@ components: // localSchema=false means the location came from a payload's @context -- // the only way the production caller passes it. A local file is not // something the network may ask this process to open, so it is refused. - doc, err := cache.loadSchemaFromPath(ctx, tmpFile.Name(), 1*time.Hour, 30*time.Second, false) + doc, err := cache.loadSchemaFromPath(ctx, tmpFile.Name(), 1*time.Hour, 30*time.Second, nil, false) if err == nil { t.Fatal("a payload-directed load opened a local file") } @@ -493,7 +493,7 @@ components: // localSchema=true is an operator naming a path in the adapter's own // config, which is the one case where opening a file is the intent. - doc, err = cache.loadSchemaFromPath(ctx, tmpFile.Name(), 1*time.Hour, 30*time.Second, true) + doc, err = cache.loadSchemaFromPath(ctx, tmpFile.Name(), 1*time.Hour, 30*time.Second, nil, true) assert.NoError(t, err) assert.NotNil(t, doc) assert.Equal(t, "3.1.0", doc.OpenAPI) @@ -517,10 +517,10 @@ info: tmpFile.Write([]byte(schemaContent)) tmpFile.Close() - doc1, err := cache.loadSchemaFromPath(ctx, tmpFile.Name(), 1*time.Hour, 30*time.Second, true) + doc1, err := cache.loadSchemaFromPath(ctx, tmpFile.Name(), 1*time.Hour, 30*time.Second, nil, true) assert.NoError(t, err) - doc2, err := cache.loadSchemaFromPath(ctx, tmpFile.Name(), 1*time.Hour, 30*time.Second, true) + doc2, err := cache.loadSchemaFromPath(ctx, tmpFile.Name(), 1*time.Hour, 30*time.Second, nil, true) assert.NoError(t, err) assert.Equal(t, doc1, doc2) @@ -530,7 +530,7 @@ func TestLoadSchemaFromPath_InvalidPath(t *testing.T) { cache := newSchemaCache(10) ctx := context.Background() - _, err := cache.loadSchemaFromPath(ctx, "/nonexistent/schema.yaml", 1*time.Hour, 30*time.Second, false) + _, err := cache.loadSchemaFromPath(ctx, "/nonexistent/schema.yaml", 1*time.Hour, 30*time.Second, nil, false) assert.Error(t, err) } @@ -559,7 +559,7 @@ components: tmpFile.Write([]byte(schemaContent)) tmpFile.Close() - doc, err := cache.loadSchemaFromPath(ctx, tmpFile.Name(), 1*time.Hour, 30*time.Second, true) + doc, err := cache.loadSchemaFromPath(ctx, tmpFile.Name(), 1*time.Hour, 30*time.Second, nil, true) assert.NoError(t, err) schema, err := findSchemaByType(ctx, doc, "TestType") @@ -589,7 +589,7 @@ components: tmpFile.Write([]byte(schemaContent)) tmpFile.Close() - doc, err := cache.loadSchemaFromPath(ctx, tmpFile.Name(), 1*time.Hour, 30*time.Second, true) + doc, err := cache.loadSchemaFromPath(ctx, tmpFile.Name(), 1*time.Hour, 30*time.Second, nil, true) assert.NoError(t, err) _, err = findSchemaByType(ctx, doc, "NonExistentType") @@ -1209,7 +1209,7 @@ components: cache.rawSchemas["TestType/attributes.yaml"] = []byte(schemaContent) - doc, err := cache.loadSchemaFromPath(ctx, "TestType/attributes.yaml", 1*time.Hour, 30*time.Second, true) + doc, err := cache.loadSchemaFromPath(ctx, "TestType/attributes.yaml", 1*time.Hour, 30*time.Second, nil, true) assert.NoError(t, err) assert.NotNil(t, doc) assert.Equal(t, "3.1.0", doc.OpenAPI) @@ -1223,7 +1223,7 @@ func TestLoadSchemaFromPath_LRUHit(t *testing.T) { cache.set(hashURL("TestType/attributes.yaml"), expected, 1*time.Hour) // localSchema=false skips rawSchemas step, goes straight to LRU - doc, err := cache.loadSchemaFromPath(ctx, "TestType/attributes.yaml", 1*time.Hour, 30*time.Second, false) + doc, err := cache.loadSchemaFromPath(ctx, "TestType/attributes.yaml", 1*time.Hour, 30*time.Second, nil, false) assert.NoError(t, err) assert.Equal(t, expected, doc) } @@ -1243,7 +1243,7 @@ info: tmpFile.Close() // rawSchemas empty, localSchema=true — local miss, falls through to file load - doc, err := cache.loadSchemaFromPath(ctx, tmpFile.Name(), 1*time.Hour, 30*time.Second, true) + doc, err := cache.loadSchemaFromPath(ctx, tmpFile.Name(), 1*time.Hour, 30*time.Second, nil, true) assert.NoError(t, err) assert.NotNil(t, doc) } @@ -1367,7 +1367,7 @@ func TestLoadSchemaFromPath_TTLExpiry_FetchesFresh(t *testing.T) { ctx := context.Background() // Load v1 with a 1ms TTL so the LRU entry expires almost immediately. - doc1, err := cache.loadSchemaFromPath(ctx, server.URL, 1*time.Millisecond, 30*time.Second, false) + doc1, err := cache.loadSchemaFromPath(ctx, server.URL, 1*time.Millisecond, 30*time.Second, nil, false) assert.NoError(t, err) assert.Equal(t, "Schema v1", doc1.Info.Title) @@ -1376,7 +1376,7 @@ func TestLoadSchemaFromPath_TTLExpiry_FetchesFresh(t *testing.T) { serveV2.Store(true) // Re-load — LRU miss (expired), freshReadFromURI fetches from the server and gets v2. - doc2, err := cache.loadSchemaFromPath(ctx, server.URL, 1*time.Hour, 30*time.Second, false) + doc2, err := cache.loadSchemaFromPath(ctx, server.URL, 1*time.Hour, 30*time.Second, nil, false) assert.NoError(t, err) assert.Equal(t, "Schema v2", doc2.Info.Title, "expected v2 after TTL expiry — global URIMapCache not bypassed") } @@ -1871,13 +1871,17 @@ func TestValidateReferencedObject_RefusesARefThatWouldReadTheDisk(t *testing.T) } } -// The packs pull 15 documents across 3 hosts -- the one the allowlist names -// plus two external spec hosts the packs $ref into -- so a $ref to a host -// outside the allowlist is the normal case, not the attack. This pins that: -// applying isAllowedDomain to $refs as well would need all three hosts named -// in the allowlist first, and would otherwise stop every pack loading. -// Deliberate, not missed. -func TestValidateReferencedObject_AllowsARefToAHostOutsideTheAllowlist(t *testing.T) { +// A $ref may not reach a host the allowlist does not name. +// +// The entry @context being allowlisted is not enough. The document it returns +// is NOT trusted -- it came from a URL the payload chose, on a host anyone can +// publish to -- so its $refs used to reach any http host at all. That let a +// payload name an attacker's document and have this process fetch whatever +// that document pointed at: an internal service, a cloud metadata endpoint. +// +// This is the case the allowlist has to cover to mean anything, because the +// refs are the great majority of the reads: one pack pulls 13-16 documents. +func TestValidateReferencedObject_RefusesARefToAHostOutsideTheAllowlist(t *testing.T) { borrowed := serveSchema(t, borrowedSchema) entry := serveSchema(t, strings.Replace(entrySchemaRefTemplate, "REF_TARGET", borrowed.URL+"/borrowed.yaml", 1)) @@ -1898,13 +1902,47 @@ func TestValidateReferencedObject_AllowsARefToAHostOutsideTheAllowlist(t *testin Path: "message.test", Context: entry.URL + "/context.jsonld", Type: "TestType", + Types: []string{"TestType"}, + Data: map[string]interface{}{"field1": "value1"}, + } + + // Only the entry host is allowlisted. The $ref host is not. + err = cache.validateReferencedObject(context.Background(), obj, + 1*time.Hour, 30*time.Second, []string{entryHost.Host}, false) + if err == nil { + t.Fatal("the cross-host $ref was fetched; a payload can point this process at any http host") + } + assert.Contains(t, err.Error(), "not in extendedSchema_allowedDomains") +} + +// And naming both hosts loads it, which is the case the packs need: a +// capability pack $refs schema.beckn.io, which $refs schema.nfh.global, so the +// allowlist has to carry every host in the chain or nothing loads. +func TestValidateReferencedObject_AllowsARefWhenBothHostsAreAllowlisted(t *testing.T) { + borrowed := serveSchema(t, borrowedSchema) + entry := serveSchema(t, strings.Replace(entrySchemaRefTemplate, "REF_TARGET", borrowed.URL+"/borrowed.yaml", 1)) + + entryHost, err := url.Parse(entry.URL) + if err != nil { + t.Fatalf("failed to parse the test server URL: %v", err) + } + borrowedHost, err := url.Parse(borrowed.URL) + if err != nil { + t.Fatalf("failed to parse the test server URL: %v", err) + } + + cache := newSchemaCache(10) + obj := referencedObject{ + Path: "message.test", + Context: entry.URL + "/context.jsonld", + Type: "TestType", + Types: []string{"TestType"}, Data: map[string]interface{}{"field1": "value1"}, } - // Only the entry host is allowlisted; the $ref host is not. if err := cache.validateReferencedObject(context.Background(), obj, - 1*time.Hour, 30*time.Second, []string{entryHost.Host}, false); err != nil { - t.Fatalf("a cross-host $ref must still resolve, or no pack can load: %v", err) + 1*time.Hour, 30*time.Second, []string{entryHost.Host, borrowedHost.Host}, false); err != nil { + t.Fatalf("both hosts allowlisted, so the chain must load: %v", err) } } @@ -1934,7 +1972,7 @@ func TestPayloadDirectedReader(t *testing.T) { t.Fatalf("failed to parse %q: %v", raw, err) } - data, err := payloadDirectedReader(openapi3.NewLoader(), u) + data, err := payloadDirectedReader(nil, false)(openapi3.NewLoader(), u) if tt.refused { if err == nil { t.Fatalf("%q was read, and must not have been", raw) From cca7ff2b739f0ea73a9c5cff66549c2cb4504112 Mon Sep 17 00:00:00 2001 From: ameersohel45 Date: Tue, 8 Sep 2026 17:55:22 +0530 Subject: [PATCH 10/10] docs(MandiPrice): follow the rename in a cross-reference [#16] The prerequisites note pointed at weather/prerequisites.go, which this branch renamed to WeatherObservation/. It was the last stale plugin path in a Go comment. --- pkg/plugin/implementation/MandiPrice/prerequisites.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/plugin/implementation/MandiPrice/prerequisites.go b/pkg/plugin/implementation/MandiPrice/prerequisites.go index d583092c..d7c1f8aa 100644 --- a/pkg/plugin/implementation/MandiPrice/prerequisites.go +++ b/pkg/plugin/implementation/MandiPrice/prerequisites.go @@ -15,6 +15,6 @@ import "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/upstr // code, a token to exchange, a point to turn into a market. Each of those is a // different upstream than the one this was written against, and each would // bring the question of where the provider-to-function binding belongs -- see -// the note in weather/prerequisites.go and prefer keeping the payload explicit +// the note in WeatherObservation/prerequisites.go and prefer keeping the payload explicit // over adding an entry here. var prerequisites = upstream.Prerequisites{}