Add provider_trait field to RuleType.Definition by intelligent-ears · Pull Request #6669 · mindersec/minder

intelligent-ears · 2026-08-02T19:41:27Z

Summary

Adds a provider_trait field to RuleType.Definition, letting a rule type declare an API/interface requirement (e.g. github, git, oci) rather than being pinned to a specific named provider registration. This reuses the existing ProviderType / CanImplement trait vocabulary already used elsewhere in the provider system, instead of introducing a
new capability abstraction.

This is currently blocking osps-ac-03-02 (branch deletion protection), since GitHub and GitLab branch-protection semantics/APIs differ enough that a single generic rule type can't cleanly cover both. This PR lays the schema + validation groundwork; applying it to unblock osps-ac-03-02 is a separate follow-up in minder-rules-and-profiles.

Changes

Added provider_trait (ProviderType, field 8) to RuleType.Definition in the proto, and regenerated bindings (pkg/api/protobuf/go/minder/v1/minder.pb.go, pkg/api/openapi/minder/v1/minder.swagger.json, docs/docs/ref/proto.mdx).
Wired validation into pkg/profiles/validator.go's validateEntities,using qtx.ListProvidersByProjectID + db.Provider.CanImplement to check whether any provider in the project satisfies the rule type's declared trait. This follows the same static, up-front pattern as the existing in_entity check, rather than the noisier ErrEvaluationSkipped per-entity skip path used by profile selectors.
Field is intentionally singular (not a list) — no existing rule type needs "trait A OR trait B"; rule types are already split per-provider (github/ vs gitlab/ in minder-rules-and-profiles) where the APIs genuinely diverge.

Testing

Added unit tests in pkg/profiles/validator_test.go covering:

A rule type's provider_trait is satisfied by a provider in the project → validation passes.
No provider in the project satisfies the declared trait → validation fails with a clear error.
Project has no providers at all, but the rule type requires a trait → validation fails.
All pre-existing test scenarios (using rule types with an unset provider_trait) continue to pass unmodified, confirming the default (PROVIDER_TYPE_UNSPECIFIED) is not gated.

Ran: go build ./pkg/profiles/... and
go test ./pkg/profiles/... -run TestValidatorScenarios -v — all 16 scenarios pass.

Scope

This covers schema + validation + tests only. Follow-ups:

rtengine-level gating, if further design discussion determines it's needed beyond static profile validation
Applying provider_trait to osps-ac-03-02
Docs for rule type authors

Part of #6650, closes #6651

CLAassistant · 2026-08-02T19:41:34Z

All committers have signed the CLA.

coveralls · 2026-08-02T19:54:10Z

coverage: 61.669% (+0.02%) from 61.654% — intelligent-ears:feat/provider-traits into mindersec:main

JAORMX · 2026-08-03T07:20:05Z

seems there are linter and unit test issues to solve

krrish175-byte · 2026-08-03T09:12:19Z

 	minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1"
 	"github.com/mindersec/minder/pkg/engine/selectors"
 	"github.com/mindersec/minder/pkg/ruletypes"
+	"github.com/mindersec/minder/internal/providers"


The internal/providers import needs to be placed above the pkg/ imports in this block. gci expects module-local internal/ packages to come before pkg/ packages, alphabetically, and the current ordering is what's tripping up the lint CI check. Moving it up should be a one-line fix.

krrish175-byte

The design here is genuinely well thought out. Reusing the existing ProviderType/CanImplement vocabulary instead of spinning up a new capability abstraction is the right call, and doing the check up front at profile validation time (matching the in_entity pattern) is much cleaner than reaching for ErrEvaluationSkipped. The test coverage covers the meaningful cases.

One thing blocking merge right now: the internal/providers import in validator.go is in the wrong group, which is causing the gci lint failure in CI. Left an inline comment with details. It's a one-line fix.

Separately, the unit test failure in TestHandleGitHubWebHook/create ("no event received after waiting 10ms") is in a completely different package and looks like a pre-existing flaky test rather than a regression from this PR. Worth getting a clean re-run after fixing the import to confirm nothing else is red.

evankanderson

This is a good start, but a couple comments for when you're back from break.

evankanderson · 2026-08-03T14:18:17Z

+        // evaluate this rule type. If PROVIDER_TYPE_UNSPECIFIED (the
+        // zero value / default), the rule type is not gated by
+        // provider trait.
+        ProviderType provider_trait = 8;


Given the presence of datasources which allow pulling from additional APIs (e.g. git and github or git and gitlab), let's make this a list -- see my last comments on #6651 for a couple examples that check both repo contents and forge-specific APIs.

evankanderson · 2026-08-03T14:28:03Z

+		if trait := ruleTypePB.Def.ProviderTrait; trait != minderv1.ProviderType_PROVIDER_TYPE_UNSPECIFIED {
+			dbTraits, err := providers.PBProviderTypesToDB([]minderv1.ProviderType{trait})
+			if err != nil {
+				return fmt.Errorf("invalid provider_trait on rule type %s: %w", ruleTypePB.Name, err)
+			}
+
+			satisfied := false
+			for _, p := range dbProviders {
+				if p.CanImplement(dbTraits[0]) {
+					satisfied = true
+					break
+				}
+			}
+
+			if !satisfied {
+				return &RuleValidationError{
+					Err: fmt.Sprintf(
+						"rule type %s requires provider trait %s, but no provider in this project implements it",
+						ruleTypePB.Name, trait.String()),
+					RuleType: ruleTypePB.Name,
+				}
+			}
+		}


I'm not sure we want to block ruletype creation / management based on the set of currently-registered providers.

Two concerns about this semantic:

It prevents loading a set of ruletypes first into an empty-ish project, then adding resources to be managed by those ruletypes. I'm not sure how common this is, but I can certainly imagine it for both automated testing and consulting-type roles.

As implemented, this protection is incomplete -- if the provider is later un-registered, the ruletype (and any profiles including it) will still remain. Unwinding that pile of policy seems surprising, so I'd probably steer away from the semantic that "you must currently have providers registered matching a ruletype's provider_traits to manage that ruletype".

I think a warning when editing a profile could be appropriate, but you'd need to add a bunch of plumbing to support collecting and returning warnings in addition to the rule execution (unlike errors, which can early-return in many cases).

With that said, I think we do want a validation that the provider_traits are in the set registered by Minder.

This should happen when the ruletype is created or updated, not when it is added to a profile.

intelligent-ears · 2026-08-06T16:57:15Z

Thanks both — makes sense on all counts.

@krrish175-byte the import ordering is fixed locally (gci), will push once I'm back.

@evankanderson agreed on both:

Switching back to a list (repeated ProviderType provider_traits), but for AND semantics this time given datasources needing e.g. git + forge-specific API together — different case from the OR-across-vendors question we closed out on the issue.
Agreed the current profile-time check against currently-registered providers has real problems (blocks preloading rule types, goes stale if a provider is removed). Will move validation to rule type create/update time instead, checking declared traits against the valid ProviderType set rather than the project's current providers, and drop the ListProvidersByProjectID check from validator.go entirely.

I'm on a break this week so will push the rework once I'm back — wanted to confirm I've got the plan right before I dig in. Will follow up on #6651 too.

intelligent-ears requested a review from a team as a code owner August 2, 2026 19:41

intelligent-ears added 3 commits August 2, 2026 15:43

Regenerate protobuf bindings for provider_trait

7cdd466

Gate rule type validation on provider_trait

fad060f

Add tests for provider_trait validation

417f0a0

intelligent-ears force-pushed the feat/provider-traits branch from 6f0b4b8 to 417f0a0 Compare August 2, 2026 19:44

JAORMX approved these changes Aug 3, 2026

View reviewed changes

krrish175-byte reviewed Aug 3, 2026

View reviewed changes

krrish175-byte requested changes Aug 3, 2026

View reviewed changes

evankanderson reviewed Aug 3, 2026

View reviewed changes

Navigation Menu

Uh oh!

Add provider_trait field to RuleType.Definition - #6669

Add provider_trait field to RuleType.Definition#6669
intelligent-ears wants to merge 3 commits into
mindersec:mainfrom
intelligent-ears:feat/provider-traits

intelligent-ears commented Aug 2, 2026

Uh oh!

CLAassistant commented Aug 2, 2026 •

edited

Loading

Uh oh!

coveralls commented Aug 2, 2026

Uh oh!

JAORMX commented Aug 3, 2026

Uh oh!

krrish175-byte Aug 3, 2026

Uh oh!

krrish175-byte left a comment

Uh oh!

evankanderson left a comment

Uh oh!

evankanderson Aug 3, 2026

Uh oh!

evankanderson Aug 3, 2026

Uh oh!

evankanderson Aug 3, 2026

Uh oh!

intelligent-ears commented Aug 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

intelligent-ears commented Aug 2, 2026

Summary

Changes

Testing

Scope

Uh oh!

CLAassistant commented Aug 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Aug 2, 2026

Uh oh!

JAORMX commented Aug 3, 2026

Uh oh!

krrish175-byte Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

krrish175-byte left a comment

Choose a reason for hiding this comment

Uh oh!

evankanderson left a comment

Choose a reason for hiding this comment

Uh oh!

evankanderson Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

evankanderson Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

evankanderson Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

intelligent-ears commented Aug 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

CLAassistant commented Aug 2, 2026 •

edited

Loading