Skip to content

Commit a4e7fd5

Browse files
patjlmclaude
andcommitted
ci-operator-checkconfig: add audience ownership validation
Add an allowed-audiences-config mechanism that restricts which org/repo combinations can use specific service account token audiences. This prevents unauthorized CI configs from using audiences belonging to other teams' WIF pools. Follows the existing cluster profile ownership pattern: - Config file maps audiences to allowed org/repo owners - ci-operator-checkconfig validates at PR time - Unlisted audiences are unrestricted (permissive by default) - Missing config file is treated as empty (no restrictions) New flag: --allowed-audiences-config (optional) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 1812ad4 commit a4e7fd5

6 files changed

Lines changed: 93 additions & 9 deletions

File tree

cmd/ci-operator-checkconfig/main.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,20 +36,23 @@ type options struct {
3636
clusterProfiles api.ClusterProfilesMap
3737
clusterClaimOwners api.ClusterClaimOwnersMap
3838
clusterProfileSetDetails validation.ClusterProfileSetDetails
39+
allowedAudiences api.AllowedAudiencesMap
3940
}
4041

4142
func (o *options) parse() error {
4243
var registryDir string
4344
var profilesConfigPath string
4445
var clusterClaimConfigPath string
4546
var clusterProfileSetDetailsPath string
47+
var allowedAudiencesConfigPath string
4648

4749
fs := flag.NewFlagSet("", flag.ExitOnError)
4850

4951
fs.StringVar(&registryDir, "registry", "", "Path to the step registry directory")
5052
fs.StringVar(&profilesConfigPath, "cluster-profiles-config", "", "Path to the cluster profile config file")
5153
fs.StringVar(&clusterClaimConfigPath, "cluster-claim-owners-config", "", "Path to the cluster claim owners config file")
5254
fs.StringVar(&clusterProfileSetDetailsPath, "cluster-profile-set-details", "", "Path to the cluster profile set details file")
55+
fs.StringVar(&allowedAudiencesConfigPath, "allowed-audiences-config", "", "Path to the allowed audiences config file")
5356
o.Options.Bind(fs)
5457

5558
if err := fs.Parse(os.Args[1:]); err != nil {
@@ -72,6 +75,14 @@ func (o *options) parse() error {
7275
}
7376
o.clusterClaimOwners = claimOwners
7477

78+
if allowedAudiencesConfigPath != "" {
79+
allowedAudiences, err := load.AllowedAudiencesConfig(allowedAudiencesConfigPath)
80+
if err != nil {
81+
return fmt.Errorf("failed to load allowed audiences config: %w", err)
82+
}
83+
o.allowedAudiences = allowedAudiences
84+
}
85+
7586
ciOPConfigAgent, err := agents.NewConfigAgent(o.ConfigDir, nil, agents.WithOrg(o.Org), agents.WithRepo(o.Repo))
7687
if err != nil {
7788
return fmt.Errorf("failed to create CI Op config agent: %w", err)
@@ -110,7 +121,8 @@ func (o *options) validate() (ret []error) {
110121
errCh := make(chan error)
111122
map_ := func() error {
112123
validator := validation.NewValidator(o.clusterProfiles, o.clusterClaimOwners,
113-
validation.WithClusterProfileSetDetails(o.clusterProfileSetDetails))
124+
validation.WithClusterProfileSetDetails(o.clusterProfileSetDetails),
125+
validation.WithAllowedAudiences(o.allowedAudiences))
114126
for c := range inputCh {
115127
if err := o.validateConfiguration(&validator, outputCh, c); err != nil {
116128
errCh <- fmt.Errorf("failed to validate configuration %s: %w", c.Metadata.RelativePath(), err)

pkg/api/types.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2996,6 +2996,21 @@ type ClusterClaimOwnerDetails struct {
29962996
Repos []string `yaml:"repos,omitempty"`
29972997
}
29982998

2999+
// AllowedAudiencesMap maps audience strings to their ownership details.
3000+
// Audiences in this map are restricted to configs from the listed org/repo owners.
3001+
// Audiences not in this map are unrestricted.
3002+
type AllowedAudiencesMap map[string]AllowedAudienceDetails
3003+
3004+
type AllowedAudienceDetails struct {
3005+
Audience string `yaml:"audience" json:"audience"`
3006+
Owners []AllowedAudienceOwners `yaml:"owners,omitempty" json:"owners,omitempty"`
3007+
}
3008+
3009+
type AllowedAudienceOwners struct {
3010+
Org string `yaml:"org" json:"org"`
3011+
Repos []string `yaml:"repos,omitempty" json:"repos,omitempty"`
3012+
}
3013+
29993014
const (
30003015
EphemeralClusterTestDoneSignalSecretName = "test-done-signal"
30013016
)

pkg/load/load.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,3 +309,25 @@ func ClusterClaimOwnersConfig(configPath string) (api.ClusterClaimOwnersMap, err
309309
}
310310
return clusterClaimOwnersMap, nil
311311
}
312+
313+
// AllowedAudiencesConfig loads allowed audiences information from its config in the release repository.
314+
// If the file does not exist, an empty map is returned.
315+
func AllowedAudiencesConfig(configPath string) (api.AllowedAudiencesMap, error) {
316+
configContents, err := os.ReadFile(configPath)
317+
if err != nil {
318+
if os.IsNotExist(err) {
319+
return make(api.AllowedAudiencesMap), nil
320+
}
321+
return nil, fmt.Errorf("failed to read allowed audiences config: %w", err)
322+
}
323+
324+
var audiencesList []api.AllowedAudienceDetails
325+
if err = yaml.Unmarshal(configContents, &audiencesList); err != nil {
326+
return nil, fmt.Errorf("failed to unmarshall allowed audiences config: %w", err)
327+
}
328+
allowedAudiencesMap := make(api.AllowedAudiencesMap)
329+
for _, a := range audiencesList {
330+
allowedAudiencesMap[a.Audience] = a
331+
}
332+
return allowedAudiencesMap, nil
333+
}

pkg/validation/config.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ type ClusterProfileSetDetails map[api.ClusterProfile][]string
2424
type Validator struct {
2525
validClusterProfiles api.ClusterProfilesMap
2626
validClusterClaimOwners api.ClusterClaimOwnersMap
27+
allowedAudiences api.AllowedAudiencesMap
2728
// hasTrapCache avoids redundant regexp searches on step commands.
2829
hasTrapCache map[string]bool
2930
cpsDetails ClusterProfileSetDetails
@@ -35,6 +36,10 @@ func WithClusterProfileSetDetails(cpsDetails ClusterProfileSetDetails) func(*Val
3536
return func(v *Validator) { v.cpsDetails = cpsDetails }
3637
}
3738

39+
func WithAllowedAudiences(audiences api.AllowedAudiencesMap) func(*Validator) {
40+
return func(v *Validator) { v.allowedAudiences = audiences }
41+
}
42+
3843
// NewValidator creates an object that optimizes bulk validations.
3944
func NewValidator(profiles api.ClusterProfilesMap, clusterClaimOwners api.ClusterClaimOwnersMap, opts ...ValidatorOption) Validator {
4045
v := Validator{
@@ -700,6 +705,6 @@ func Observer(observer api.Observer) []error {
700705
// this observer in the future. This technically disallows users from using `from:`
701706
// to refer to an image from a release payload for an observer, but this should be
702707
// not of any real issue and will at least be obvious to the user on presubmit.
703-
errs = append(errs, validateFromAndFromImage(newContext("", nil, nil, nil), observer.From, observer.FromImage, nil, nil)...)
708+
errs = append(errs, validateFromAndFromImage(newContext("", nil, nil, nil, nil), observer.From, observer.FromImage, nil, nil)...)
704709
return errs
705710
}

pkg/validation/test.go

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,8 @@ type context struct {
7373
inputImagesSeen testInputImages
7474
// releases is used to validate references to release images .
7575
releases sets.Set[string]
76+
// metadata is used for ownership checks (e.g., audience restrictions).
77+
metadata *api.Metadata
7678
}
7779

7880
// newContext creates a top-level context.
@@ -81,6 +83,7 @@ func newContext(
8183
env api.TestEnvironment,
8284
releases sets.Set[string],
8385
inputImagesSeen testInputImages,
86+
metadata *api.Metadata,
8487
) *context {
8588
return &context{
8689
field: field,
@@ -89,6 +92,7 @@ func newContext(
8992
leasesSeen: sets.New[string](),
9093
inputImagesSeen: inputImagesSeen,
9194
releases: releases,
95+
metadata: metadata,
9296
}
9397
}
9498

@@ -575,7 +579,7 @@ func (v *Validator) validateTestConfigurationType(
575579
clusterCount++
576580
validationErrors = append(validationErrors, v.validateClusterProfile(fieldRoot, testConfig.ClusterProfile, metadata)...)
577581
}
578-
context := newContext(fieldPath(fieldRoot), testConfig.Environment, releases, inputImagesSeen)
582+
context := newContext(fieldPath(fieldRoot), testConfig.Environment, releases, inputImagesSeen, metadata)
579583
validationErrors = append(validationErrors, validateLeases(context.addField("leases"), testConfig.Leases)...)
580584
if testConfig.NodeArchitecture != nil {
581585
validationErrors = append(validationErrors, validateNodeArchitecture(fieldRoot, *testConfig.NodeArchitecture))
@@ -586,7 +590,7 @@ func (v *Validator) validateTestConfigurationType(
586590
}
587591
if testConfig := test.MultiStageTestConfigurationLiteral; testConfig != nil {
588592
typeCount++
589-
context := newContext(fieldPath(fieldRoot).addField("steps"), testConfig.Environment, releases, inputImagesSeen)
593+
context := newContext(fieldPath(fieldRoot).addField("steps"), testConfig.Environment, releases, inputImagesSeen, metadata)
590594
if testConfig.ClusterProfile != "" {
591595
clusterCount++
592596
validationErrors = append(validationErrors, v.validateClusterProfile(fieldRoot, testConfig.ClusterProfile, metadata)...)
@@ -695,7 +699,7 @@ func (v *Validator) validateLiteralTestStep(context *context, stage testStage, s
695699
}
696700
ret = append(ret, validateDependencies(string(context.field), step.Dependencies)...)
697701
ret = append(ret, validateLeases(context.addField("leases"), step.Leases)...)
698-
ret = append(ret, validateServiceAccountTokens(string(context.field), step.ServiceAccountTokens)...)
702+
ret = append(ret, v.validateServiceAccountTokens(string(context.field), step.ServiceAccountTokens, context.metadata)...)
699703
if step.NodeArchitecture != nil {
700704
if err := validateNodeArchitecture(string(context.field), *step.NodeArchitecture); err != nil {
701705
ret = append(ret, err)
@@ -943,12 +947,18 @@ func validateLeases(context *context, leases []api.StepLease) (ret []error) {
943947
return
944948
}
945949

946-
func validateServiceAccountTokens(fieldRoot string, tokens []api.ServiceAccountTokenVolume) (ret []error) {
950+
func (v *Validator) validateServiceAccountTokens(fieldRoot string, tokens []api.ServiceAccountTokenVolume, metadata *api.Metadata) (ret []error) {
947951
mountPaths := sets.New[string]()
948952
for i, token := range tokens {
949953
fieldPath := fmt.Sprintf("%s.service_account_tokens[%d]", fieldRoot, i)
950954
if token.Audience == "" {
951955
ret = append(ret, fmt.Errorf("%s.audience: must not be empty", fieldPath))
956+
} else if v.allowedAudiences != nil {
957+
if details, ok := v.allowedAudiences[token.Audience]; ok {
958+
if err := verifyAudienceOwnership(details, metadata); err != nil {
959+
ret = append(ret, err)
960+
}
961+
}
952962
}
953963
if token.MountPath == "" {
954964
ret = append(ret, fmt.Errorf("%s.mount_path: must not be empty", fieldPath))
@@ -964,6 +974,26 @@ func validateServiceAccountTokens(fieldRoot string, tokens []api.ServiceAccountT
964974
return
965975
}
966976

977+
// verifyAudienceOwnership checks if metadata's org and repo match those in the
978+
// audience config, verifying if it's one of the owners of the audience.
979+
func verifyAudienceOwnership(audience api.AllowedAudienceDetails, m *api.Metadata) error {
980+
if m == nil || m.Org == "" {
981+
return nil
982+
}
983+
if len(audience.Owners) == 0 {
984+
return nil
985+
}
986+
for _, owner := range audience.Owners {
987+
if owner.Org != m.Org {
988+
continue
989+
}
990+
if owner.Repos == nil || util.Contains(owner.Repos, m.Repo) {
991+
return nil
992+
}
993+
}
994+
return fmt.Errorf("%s/%s is not allowed to use service account token audience %q", m.Org, m.Repo, audience.Audience)
995+
}
996+
967997
func validateNodeArchitectureOverrides(fieldRoot string, nodeArchitectureOverrides api.NodeArchitectureOverrides) error {
968998
for index, arch := range nodeArchitectureOverrides {
969999
if err := arch.Validate(); err != nil {

pkg/validation/test_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1094,7 +1094,7 @@ func TestValidateTestSteps(t *testing.T) {
10941094
clusterClaim: api.ClaimRelease{ReleaseName: "myclaim-as", OverrideName: "myclaim"},
10951095
}} {
10961096
t.Run(tc.name, func(t *testing.T) {
1097-
context := newContext("test", nil, tc.releases, make(testInputImages))
1097+
context := newContext("test", nil, tc.releases, make(testInputImages), nil)
10981098
if tc.seen != nil {
10991099
context.namesSeen = tc.seen
11001100
}
@@ -1135,7 +1135,7 @@ func TestValidatePostSteps(t *testing.T) {
11351135
}},
11361136
}} {
11371137
t.Run(tc.name, func(t *testing.T) {
1138-
context := newContext("test", nil, tc.releases, make(testInputImages))
1138+
context := newContext("test", nil, tc.releases, make(testInputImages), nil)
11391139
if tc.seen != nil {
11401140
context.namesSeen = tc.seen
11411141
}
@@ -1173,7 +1173,7 @@ func TestValidateParameters(t *testing.T) {
11731173
}} {
11741174
t.Run(tc.name, func(t *testing.T) {
11751175
v := NewValidator(nil, nil)
1176-
err := v.validateLiteralTestStep(newContext("test", tc.env, tc.releases, make(testInputImages)), testStageTest, api.LiteralTestStep{
1176+
err := v.validateLiteralTestStep(newContext("test", tc.env, tc.releases, make(testInputImages), nil), testStageTest, api.LiteralTestStep{
11771177
As: "as",
11781178
From: "from",
11791179
Commands: "commands",

0 commit comments

Comments
 (0)