From 090ce5ae2c06c9d27854abdd1c3a0c99fd4d3772 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Mon, 5 Oct 2026 11:20:21 +0200 Subject: [PATCH 1/2] fix(github-config): refuse a managed team that names a parent team or directory group A nested team inherits its parent's repository access on GitHub, outside the grant rules that cap what the github-config release may hand out, and a directory link moves membership outside the member allow-list. The Team rules checked neither. Refuse all four fields the provider exposes, in forProvider and initProvider, with one literal JMESPath comparison. Fixes #4511 Co-Authored-By: Claude Opus 5.5 --- .../restrict-github-team-management.yaml | 52 ++++++- tests/policy-failure-actions.json | 1 + .../kyverno-test.yaml | 11 ++ .../kyverno-test.yaml | 33 +++++ .../nested-or-directory-linked/resources.yaml | 129 ++++++++++++++++++ 5 files changed, 225 insertions(+), 1 deletion(-) create mode 100644 tests/restrict-github-team-management/nested-or-directory-linked/kyverno-test.yaml create mode 100644 tests/restrict-github-team-management/nested-or-directory-linked/resources.yaml diff --git a/k8s/bases/infrastructure/cluster-policies/best-practices/restrict-github-team-management.yaml b/k8s/bases/infrastructure/cluster-policies/best-practices/restrict-github-team-management.yaml index c35becf3cb..5cfae421ba 100644 --- a/k8s/bases/infrastructure/cluster-policies/best-practices/restrict-github-team-management.yaml +++ b/k8s/bases/infrastructure/cluster-policies/best-practices/restrict-github-team-management.yaml @@ -21,7 +21,8 @@ metadata: Restricts provider-upjet-github team resources in the github-config namespace to the github-config CODEOWNERS teams, requires TeamMembership and TeamRepository resources to reference those Team objects by name, and - blocks repository admin grants. This prevents a compromised or unintended + blocks repository admin grants. A Team may not be nested under a parent + team or linked to a directory group. This prevents a compromised or unintended github-config artifact from adding arbitrary users to arbitrary GitHub teams or granting privileged repository access. spec: @@ -92,6 +93,55 @@ spec: # numeric identities from a foreign one, plus the provider ServiceAccount # excluded so its post-create write still succeeds. Tracked in #3144 rather # than half-closed here. + # + # #4511: a nested team inherits every repository grant of its parent, and + # that inheritance happens on GitHub — it never passes through the + # TeamRepository rules below, which are what cap this delegation's grants. A + # Team that named a parent would hand its members the parent's access with no + # grant for those rules to judge. The directory link is refused for the same + # reason from the other side: it lets a group outside this cluster decide who + # the members are, past the member allow-list. + # + # All four fields the provider exposes are covered, in both blocks. + # parentTeamReadId and parentTeamReadSlug are documented as read-back values, + # but the schema accepts them as input, so they are refused rather than + # trusted to stay inert. An empty string is what the provider reports for + # "none" and is accepted. + # + # Measured live: neither team sets any of them, and the provider reports all + # four empty for both. The provider is NOT exempt. If a team were nested on + # GitHub by hand, the provider would try to record the parent on the admins + # object, be refused, and report the object out of sync — which is the + # signal wanted for a nesting nobody reviewed. + - name: teams-not-nested-or-directory-linked + match: + any: + - resources: + kinds: + - team.github.m.upbound.io/*/Team + namespaces: + - github-config + validate: + failureAction: Enforce + # Kyverno admits an update by default when the object already failed the + # rule before it. No object fails it today, and one that somehow did + # must not become freely editable for that reason. + allowExistingViolations: false + message: >- + A github-config Team must not set parentTeamId, parentTeamReadId, + parentTeamReadSlug or ldapDn, in forProvider or initProvider. Nesting a + team or linking it to a directory group is a reviewed change to + restrict-github-team-management.yaml. + deny: + conditions: + all: + # ONE comparison evaluated by JMESPath rather than eight Kyverno + # operators: AnyNotIn and Equals read * and ? in an operand as + # wildcards, and these fields are written by the party this rule + # constrains. JMESPath == compares strings literally. + - key: "{{ (request.object.spec.forProvider.parentTeamId || '') == '' && (request.object.spec.forProvider.parentTeamReadId || '') == '' && (request.object.spec.forProvider.parentTeamReadSlug || '') == '' && (request.object.spec.forProvider.ldapDn || '') == '' && (request.object.spec.initProvider.parentTeamId || '') == '' && (request.object.spec.initProvider.parentTeamReadId || '') == '' && (request.object.spec.initProvider.parentTeamReadSlug || '') == '' && (request.object.spec.initProvider.ldapDn || '') == '' }}" + operator: Equals + value: false - name: teammemberships-reference-allow-listed-teams match: any: diff --git a/tests/policy-failure-actions.json b/tests/policy-failure-actions.json index 8abe84f4f7..f8a28dca25 100644 --- a/tests/policy-failure-actions.json +++ b/tests/policy-failure-actions.json @@ -6,6 +6,7 @@ "disallow-latest-tag/validate-image-tag": "Audit", "restrict-github-team-management/teams-allow-listed": "Enforce", "restrict-github-team-management/teams-bind-provider-identity-to-object-name": "Enforce", + "restrict-github-team-management/teams-not-nested-or-directory-linked": "Enforce", "restrict-github-team-management/teammemberships-reference-allow-listed-teams": "Enforce", "restrict-github-team-management/teammemberships-allow-listed-members": "Enforce", "restrict-github-team-management/teammemberships-known-roles": "Enforce", diff --git a/tests/restrict-github-team-management/kyverno-test.yaml b/tests/restrict-github-team-management/kyverno-test.yaml index 04bd3b4732..baabb5c27f 100644 --- a/tests/restrict-github-team-management/kyverno-test.yaml +++ b/tests/restrict-github-team-management/kyverno-test.yaml @@ -35,6 +35,17 @@ results: - github-config/attacker-team kind: Team result: pass + # None of the paved-road teams names a parent team or a directory group. + # attacker-team is here for the same reason as above: the nesting rule judges + # only those fields, whatever the allow-list says about the object. + - policy: restrict-github-team-management + rule: teams-not-nested-or-directory-linked + resources: + - github-config/admins + - github-config/maintainers + - github-config/attacker-team + kind: Team + result: pass # References must go through the allow-listed Team objects, never a raw ID. - policy: restrict-github-team-management rule: teammemberships-reference-allow-listed-teams diff --git a/tests/restrict-github-team-management/nested-or-directory-linked/kyverno-test.yaml b/tests/restrict-github-team-management/nested-or-directory-linked/kyverno-test.yaml new file mode 100644 index 0000000000..a20aa07301 --- /dev/null +++ b/tests/restrict-github-team-management/nested-or-directory-linked/kyverno-test.yaml @@ -0,0 +1,33 @@ +--- +apiVersion: cli.kyverno.io/v1alpha1 +kind: Test +metadata: + name: restrict-github-team-management-nested-or-directory-linked +policies: + - >- + ../../../k8s/bases/infrastructure/cluster-policies/best-practices/restrict-github-team-management.yaml +resources: + - resources.yaml +results: + - policy: restrict-github-team-management + rule: teams-not-nested-or-directory-linked + resources: + - github-config/forprovider-parentteamid + - github-config/forprovider-parentteamreadid + - github-config/forprovider-parentteamreadslug + - github-config/forprovider-ldapdn + - github-config/initprovider-parentteamid + - github-config/initprovider-parentteamreadid + - github-config/initprovider-parentteamreadslug + - github-config/initprovider-ldapdn + - github-config/wildcard-star + - github-config/wildcard-question + - github-config/admins + kind: Team + result: fail + - policy: restrict-github-team-management + rule: teams-not-nested-or-directory-linked + resources: + - github-config/maintainers + kind: Team + result: pass diff --git a/tests/restrict-github-team-management/nested-or-directory-linked/resources.yaml b/tests/restrict-github-team-management/nested-or-directory-linked/resources.yaml new file mode 100644 index 0000000000..90deb6325a --- /dev/null +++ b/tests/restrict-github-team-management/nested-or-directory-linked/resources.yaml @@ -0,0 +1,129 @@ +--- +# A parent team, a read-back parent field, or a directory link — each one alone, +# in each block the provider reads. Every one of these must be refused. The +# object names are not on the team allow-list on purpose: this fixture asserts +# only the nesting rule, which has to refuse them whatever the name. +apiVersion: team.github.m.upbound.io/v1beta1 +kind: Team +metadata: + name: forprovider-parentteamid + namespace: github-config +spec: + forProvider: + parentTeamId: "1234567" +--- +apiVersion: team.github.m.upbound.io/v1beta1 +kind: Team +metadata: + name: forprovider-parentteamreadid + namespace: github-config +spec: + forProvider: + parentTeamReadId: "1234567" +--- +apiVersion: team.github.m.upbound.io/v1beta1 +kind: Team +metadata: + name: forprovider-parentteamreadslug + namespace: github-config +spec: + forProvider: + parentTeamReadSlug: other-team +--- +apiVersion: team.github.m.upbound.io/v1beta1 +kind: Team +metadata: + name: forprovider-ldapdn + namespace: github-config +spec: + forProvider: + ldapDn: cn=owners,ou=groups,dc=example,dc=com +--- +apiVersion: team.github.m.upbound.io/v1beta1 +kind: Team +metadata: + name: initprovider-parentteamid + namespace: github-config +spec: + initProvider: + parentTeamId: "1234567" +--- +apiVersion: team.github.m.upbound.io/v1beta1 +kind: Team +metadata: + name: initprovider-parentteamreadid + namespace: github-config +spec: + initProvider: + parentTeamReadId: "1234567" +--- +apiVersion: team.github.m.upbound.io/v1beta1 +kind: Team +metadata: + name: initprovider-parentteamreadslug + namespace: github-config +spec: + initProvider: + parentTeamReadSlug: other-team +--- +apiVersion: team.github.m.upbound.io/v1beta1 +kind: Team +metadata: + name: initprovider-ldapdn + namespace: github-config +spec: + initProvider: + ldapDn: cn=owners,ou=groups,dc=example,dc=com +--- +# Kyverno's own operators read * and ? as wildcards, so a parent written as a +# wildcard is the value most likely to slip past a comparison. It is text like +# any other and must be refused. +apiVersion: team.github.m.upbound.io/v1beta1 +kind: Team +metadata: + name: wildcard-star + namespace: github-config +spec: + forProvider: + parentTeamId: "*" +--- +apiVersion: team.github.m.upbound.io/v1beta1 +kind: Team +metadata: + name: wildcard-question + namespace: github-config +spec: + initProvider: + parentTeamReadSlug: "?" +--- +# An approved forProvider block must not front a parent supplied through +# initProvider, which upjet merges into any field forProvider leaves unset. +apiVersion: team.github.m.upbound.io/v1beta1 +kind: Team +metadata: + name: admins + namespace: github-config +spec: + forProvider: + name: Admins + privacy: secret + initProvider: + parentTeamId: "1234567" +--- +# The provider reports "no parent" and "no directory link" as empty strings. +# Writing them out changes nothing on GitHub and must stay admitted. +apiVersion: team.github.m.upbound.io/v1beta1 +kind: Team +metadata: + name: maintainers + namespace: github-config +spec: + forProvider: + name: maintainers + parentTeamId: "" + parentTeamReadId: "" + parentTeamReadSlug: "" + ldapDn: "" + initProvider: + parentTeamId: "" + ldapDn: "" From 29976126292f2cbc5ae89cafa1dce5924c0d7399 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Mon, 5 Oct 2026 11:24:59 +0200 Subject: [PATCH 2/2] fix(github-config): refuse non-text parent and directory values in the rule itself Review finding: `x || ''` also reads false, [] and {} as empty, so only the provider's schema stood between those values and admission. not_null() replaces a missing value only. Adds fixtures for a boolean, an empty list, a blank string and a Team with no spec, and words the message to match what is accepted. Co-Authored-By: Claude Opus 5.5 --- .../restrict-github-team-management.yaml | 18 ++++++--- .../kyverno-test.yaml | 4 ++ .../nested-or-directory-linked/resources.yaml | 38 +++++++++++++++++++ 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/k8s/bases/infrastructure/cluster-policies/best-practices/restrict-github-team-management.yaml b/k8s/bases/infrastructure/cluster-policies/best-practices/restrict-github-team-management.yaml index 5cfae421ba..3ce08e99de 100644 --- a/k8s/bases/infrastructure/cluster-policies/best-practices/restrict-github-team-management.yaml +++ b/k8s/bases/infrastructure/cluster-policies/best-practices/restrict-github-team-management.yaml @@ -125,12 +125,15 @@ spec: failureAction: Enforce # Kyverno admits an update by default when the object already failed the # rule before it. No object fails it today, and one that somehow did - # must not become freely editable for that reason. + # must not become freely editable for that reason. The cost: such an + # object could not be changed at all, its finalizer included, until + # the field is cleared. allowExistingViolations: false message: >- - A github-config Team must not set parentTeamId, parentTeamReadId, - parentTeamReadSlug or ldapDn, in forProvider or initProvider. Nesting a - team or linking it to a directory group is a reviewed change to + A github-config Team must leave parentTeamId, parentTeamReadId, + parentTeamReadSlug and ldapDn unset or empty, in forProvider and + initProvider. Nesting a team or linking it to a directory group is a + reviewed change to restrict-github-team-management.yaml. deny: conditions: @@ -138,8 +141,11 @@ spec: # ONE comparison evaluated by JMESPath rather than eight Kyverno # operators: AnyNotIn and Equals read * and ? in an operand as # wildcards, and these fields are written by the party this rule - # constrains. JMESPath == compares strings literally. - - key: "{{ (request.object.spec.forProvider.parentTeamId || '') == '' && (request.object.spec.forProvider.parentTeamReadId || '') == '' && (request.object.spec.forProvider.parentTeamReadSlug || '') == '' && (request.object.spec.forProvider.ldapDn || '') == '' && (request.object.spec.initProvider.parentTeamId || '') == '' && (request.object.spec.initProvider.parentTeamReadId || '') == '' && (request.object.spec.initProvider.parentTeamReadSlug || '') == '' && (request.object.spec.initProvider.ldapDn || '') == '' }}" + # constrains. JMESPath == compares strings literally. not_null() + # replaces only a missing value with the empty string, so false, + # an empty list or an empty object is refused here and not left + # to the provider's schema to reject. + - key: "{{ not_null(request.object.spec.forProvider.parentTeamId, '') == '' && not_null(request.object.spec.forProvider.parentTeamReadId, '') == '' && not_null(request.object.spec.forProvider.parentTeamReadSlug, '') == '' && not_null(request.object.spec.forProvider.ldapDn, '') == '' && not_null(request.object.spec.initProvider.parentTeamId, '') == '' && not_null(request.object.spec.initProvider.parentTeamReadId, '') == '' && not_null(request.object.spec.initProvider.parentTeamReadSlug, '') == '' && not_null(request.object.spec.initProvider.ldapDn, '') == '' }}" operator: Equals value: false - name: teammemberships-reference-allow-listed-teams diff --git a/tests/restrict-github-team-management/nested-or-directory-linked/kyverno-test.yaml b/tests/restrict-github-team-management/nested-or-directory-linked/kyverno-test.yaml index a20aa07301..1321272df6 100644 --- a/tests/restrict-github-team-management/nested-or-directory-linked/kyverno-test.yaml +++ b/tests/restrict-github-team-management/nested-or-directory-linked/kyverno-test.yaml @@ -23,11 +23,15 @@ results: - github-config/wildcard-star - github-config/wildcard-question - github-config/admins + - github-config/typed-false + - github-config/typed-empty-list + - github-config/whitespace kind: Team result: fail - policy: restrict-github-team-management rule: teams-not-nested-or-directory-linked resources: - github-config/maintainers + - github-config/no-spec kind: Team result: pass diff --git a/tests/restrict-github-team-management/nested-or-directory-linked/resources.yaml b/tests/restrict-github-team-management/nested-or-directory-linked/resources.yaml index 90deb6325a..93e1296cb1 100644 --- a/tests/restrict-github-team-management/nested-or-directory-linked/resources.yaml +++ b/tests/restrict-github-team-management/nested-or-directory-linked/resources.yaml @@ -127,3 +127,41 @@ spec: initProvider: parentTeamId: "" ldapDn: "" +--- +# Not text at all. The provider's schema declares these fields as strings and +# would reject each of these first, but the rule must not depend on that: a +# value that is merely "empty-looking" is still a value somebody set. +apiVersion: team.github.m.upbound.io/v1beta1 +kind: Team +metadata: + name: typed-false + namespace: github-config +spec: + forProvider: + parentTeamId: false +--- +apiVersion: team.github.m.upbound.io/v1beta1 +kind: Team +metadata: + name: typed-empty-list + namespace: github-config +spec: + initProvider: + ldapDn: [] +--- +# Blank to a reader, not empty to GitHub. +apiVersion: team.github.m.upbound.io/v1beta1 +kind: Team +metadata: + name: whitespace + namespace: github-config +spec: + forProvider: + parentTeamReadSlug: " " +--- +# Nothing under spec at all: every field is missing, none is set. +apiVersion: team.github.m.upbound.io/v1beta1 +kind: Team +metadata: + name: no-spec + namespace: github-config