fix(github-config): refuse a managed team that names a parent team or directory group

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:
@@ -91,6 +92,61 @@ spec:
     # team's numeric identity and exempts the provider's own write. That rule
     # depends on the writing identity, so it cannot live in this
     # background-scanned policy.
+    #
+    # #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. 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 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:
+            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. 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
       match:
         any:

tests/policy-failure-actions.json

@@ -7,6 +7,7 @@
   "restrict-github-team-external-identity/teams-external-name-is-an-approved-identity": "Enforce",
   "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",

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

tests/restrict-github-team-management/nested-or-directory-linked/kyverno-test.yaml

@@ -0,0 +1,37 @@
+---
+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
+      - 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

tests/restrict-github-team-management/nested-or-directory-linked/resources.yaml

@@ -0,0 +1,167 @@
+---
+# 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: ""
+---
+# 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