From c8a604000097ce87846027a41a9d9303bb4e3326 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sat, 18 Jul 2026 22:32:54 +0200 Subject: [PATCH 1/5] fix(github-config): constrain team management --- k8s/bases/apps/github-config/role.yaml | 19 +++- .../restrict-github-team-management.yaml | 99 +++++++++++++++++++ .../cluster-policies/kustomization.yaml | 1 + 3 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 k8s/bases/infrastructure/cluster-policies/best-practices/restrict-github-team-management.yaml diff --git a/k8s/bases/apps/github-config/role.yaml b/k8s/bases/apps/github-config/role.yaml index 131fb3c6f..a7c412198 100644 --- a/k8s/bases/apps/github-config/role.yaml +++ b/k8s/bases/apps/github-config/role.yaml @@ -27,7 +27,6 @@ metadata: rules: - apiGroups: - repo.github.m.upbound.io - - team.github.m.upbound.io - actions.github.m.upbound.io - enterprise.github.m.upbound.io - github.m.upbound.io @@ -41,6 +40,24 @@ rules: - update - patch - delete + # Team resources are high-impact because they change real GitHub org + # authorization. Keep the RBAC surface explicit (no API-group wildcard) and + # rely on restrict-github-team-management Kyverno admission policy to constrain + # the allowed team names, reference style, and repository permissions. + - apiGroups: + - team.github.m.upbound.io + resources: + - teams + - teammemberships + - teamrepositories + verbs: + - get + - list + - watch + - create + - update + - patch + - delete - apiGroups: - external-secrets.io resources: 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 new file mode 100644 index 000000000..b7f64af61 --- /dev/null +++ b/k8s/bases/infrastructure/cluster-policies/best-practices/restrict-github-team-management.yaml @@ -0,0 +1,99 @@ +# Constrains the high-impact GitHub team managed resources delegated to the +# github-config tenant. The tenant's Flux Kustomization applies a separately +# published OCI artifact as the github-config ServiceAccount, and those +# Crossplane resources reconcile with an org-admin GitHub App. Namespace RBAC can +# limit the API group, but it cannot restrict resource spec fields such as the +# team, member, repository, role, or permission. This admission guard keeps that +# delegation scoped to the intended CODEOWNERS teams and blocks direct numeric +# team IDs/selectors that would bypass the allow-list enforced through +# Crossplane references. +apiVersion: kyverno.io/v1 +kind: ClusterPolicy +metadata: + name: restrict-github-team-management + annotations: + policies.kyverno.io/title: Restrict GitHub Team Management + policies.kyverno.io/category: Security, GitHub + policies.kyverno.io/severity: high + policies.kyverno.io/subject: Team, TeamMembership, TeamRepository + policies.kyverno.io/minversion: 1.6.0 + policies.kyverno.io/description: >- + Restricts provider-upjet-github team resources in the github-config + namespace to the platform 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 + github-config artifact from adding arbitrary users to arbitrary GitHub + teams or granting privileged repository access. +spec: + validationFailureAction: Enforce + background: true + rules: + - name: teams-allow-listed + match: + any: + - resources: + kinds: + - Team.team.github.m.upbound.io + namespaces: + - github-config + validate: + message: GitHub Team resources managed by github-config must be one of platform or maintainers. + deny: + conditions: + all: + - key: "{{ request.object.metadata.name }}" + operator: AnyNotIn + value: + - platform + - maintainers + - name: teammemberships-reference-allow-listed-teams + match: + any: + - resources: + kinds: + - TeamMembership.team.github.m.upbound.io + namespaces: + - github-config + validate: + message: GitHub TeamMembership resources must use spec.forProvider.teamIdRef.name set to platform or maintainers; direct teamId values and selectors are not allowed. + deny: + conditions: + any: + - key: "{{ request.object.spec.forProvider.teamId || '' }}" + operator: NotEquals + value: "" + - key: "{{ request.object.spec.forProvider.teamIdSelector || `{}` }}" + operator: NotEquals + value: {} + - key: "{{ request.object.spec.forProvider.teamIdRef.name || '' }}" + operator: AnyNotIn + value: + - platform + - maintainers + - name: teamrepositories-reference-allow-listed-teams + match: + any: + - resources: + kinds: + - TeamRepository.team.github.m.upbound.io + namespaces: + - github-config + validate: + message: GitHub TeamRepository resources must reference platform or maintainers by spec.forProvider.teamIdRef.name and may not grant admin permission. + deny: + conditions: + any: + - key: "{{ request.object.spec.forProvider.teamId || '' }}" + operator: NotEquals + value: "" + - key: "{{ request.object.spec.forProvider.teamIdSelector || `{}` }}" + operator: NotEquals + value: {} + - key: "{{ request.object.spec.forProvider.teamIdRef.name || '' }}" + operator: AnyNotIn + value: + - platform + - maintainers + - key: "{{ request.object.spec.forProvider.permission || '' }}" + operator: Equals + value: admin diff --git a/k8s/bases/infrastructure/cluster-policies/kustomization.yaml b/k8s/bases/infrastructure/cluster-policies/kustomization.yaml index 381a7b082..0b508d9d0 100644 --- a/k8s/bases/infrastructure/cluster-policies/kustomization.yaml +++ b/k8s/bases/infrastructure/cluster-policies/kustomization.yaml @@ -12,6 +12,7 @@ resources: - best-practices/disable-default-sa-automount.yaml - best-practices/disallow-latest-tag.yaml - best-practices/propagate-reloader-to-flagger-primary.yaml + - best-practices/restrict-github-team-management.yaml - best-practices/restrict-tenant-secret-stores.yaml - best-practices/validate-host-restrictions.yaml - best-practices/validate-pdb-drain-safe.yaml From 2b57d4199ef3d135946f4e9b8f09773cc438fe04 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sat, 15 Aug 2026 08:57:33 +0200 Subject: [PATCH 2/5] fix(cluster-policies): make the GitHub team guard match, and bind team identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The policy matched kinds as Team.team.github.m.upbound.io — kubectl's resource.group shorthand, not Kyverno's group/version/Kind form that every other CRD policy here uses. Nothing matched it, so an Enforce policy would have deployed protecting nothing. Fixed to team.github.m.upbound.io/*/Team, with a version wildcard so a provider bump cannot silently un-protect it. Also closes the identity gap the review raised: the allow-list checked only metadata.name while the provider reconciles spec.forProvider.name, so a Team named platform could point at any GitHub team and the teamIdRef rules would still accept it. Binds forProvider/initProvider name to the object name, and blocks foreign crossplane.io/external-name adoption at creation. Adds kyverno test fixtures covering both, including the escalation itself. --- .../restrict-github-team-management.yaml | 70 ++++++++++++++- .../external-name-adoption/kyverno-test.yaml | 31 +++++++ .../external-name-adoption/resources.yaml | 14 +++ .../forprovider-mismatch/kyverno-test.yaml | 25 ++++++ .../forprovider-mismatch/resources.yaml | 15 ++++ .../initprovider-mismatch/kyverno-test.yaml | 23 +++++ .../initprovider-mismatch/resources.yaml | 11 +++ .../kyverno-test.yaml | 72 +++++++++++++++ .../resources.yaml | 87 +++++++++++++++++++ 9 files changed, 345 insertions(+), 3 deletions(-) create mode 100644 tests/restrict-github-team-management/external-name-adoption/kyverno-test.yaml create mode 100644 tests/restrict-github-team-management/external-name-adoption/resources.yaml create mode 100644 tests/restrict-github-team-management/forprovider-mismatch/kyverno-test.yaml create mode 100644 tests/restrict-github-team-management/forprovider-mismatch/resources.yaml create mode 100644 tests/restrict-github-team-management/initprovider-mismatch/kyverno-test.yaml create mode 100644 tests/restrict-github-team-management/initprovider-mismatch/resources.yaml create mode 100644 tests/restrict-github-team-management/kyverno-test.yaml create mode 100644 tests/restrict-github-team-management/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 b7f64af61..e06e45d35 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 @@ -33,7 +33,7 @@ spec: any: - resources: kinds: - - Team.team.github.m.upbound.io + - team.github.m.upbound.io/*/Team namespaces: - github-config validate: @@ -46,12 +46,76 @@ spec: value: - platform - maintainers + # The allow-list above constrains the Kubernetes object name, but the + # provider takes the real GitHub team from spec.forProvider.name (and + # spec.initProvider.name). Without this rule a Team object named "platform" + # may carry forProvider.name: some-other-team, so the reference rules below + # accept teamIdRef.name: platform while the org-admin provider reconciles a + # different remote team. Binding the provider identity to the object name is + # what makes the object name a faithful proxy for the remote team, which is + # the assumption the two reference rules rest on. Both fields are authored + # by the tenant and never rewritten by the provider, so pinning them carries + # no reconciliation risk. + - name: teams-bind-provider-identity-to-object-name + match: + any: + - resources: + kinds: + - team.github.m.upbound.io/*/Team + namespaces: + - github-config + validate: + message: GitHub Team spec.forProvider.name and spec.initProvider.name must be omitted or equal to metadata.name, so the allow-listed object name is the team actually reconciled. + deny: + conditions: + any: + - key: "{{ request.object.spec.forProvider.name || '' }}" + operator: AnyNotIn + value: + - "" + - "{{ request.object.metadata.name }}" + - key: "{{ request.object.spec.initProvider.name || '' }}" + operator: AnyNotIn + value: + - "" + - "{{ request.object.metadata.name }}" + # crossplane.io/external-name is the other way the local name can be + # detached from the remote team: setting it at creation adopts a + # pre-existing GitHub team regardless of what metadata.name says. This is + # scoped to CREATE deliberately — provider-upjet-github writes the numeric + # team ID back into this annotation after creation, so enforcing it on + # UPDATE would block the provider's own reconciliation. Tenant-authored + # UPDATE of this annotation is therefore still open and needs the provider + # ServiceAccount excluded to close; tracked separately. + - name: teams-block-foreign-external-name-adoption + match: + any: + - resources: + kinds: + - team.github.m.upbound.io/*/Team + namespaces: + - github-config + preconditions: + all: + - key: "{{ request.operation || 'BACKGROUND' }}" + operator: Equals + value: CREATE + validate: + message: GitHub Team resources must not set crossplane.io/external-name to a different team at creation; omit it or set it to metadata.name. + deny: + conditions: + any: + - key: "{{ request.object.metadata.annotations.\"crossplane.io/external-name\" || '' }}" + operator: AnyNotIn + value: + - "" + - "{{ request.object.metadata.name }}" - name: teammemberships-reference-allow-listed-teams match: any: - resources: kinds: - - TeamMembership.team.github.m.upbound.io + - team.github.m.upbound.io/*/TeamMembership namespaces: - github-config validate: @@ -75,7 +139,7 @@ spec: any: - resources: kinds: - - TeamRepository.team.github.m.upbound.io + - team.github.m.upbound.io/*/TeamRepository namespaces: - github-config validate: diff --git a/tests/restrict-github-team-management/external-name-adoption/kyverno-test.yaml b/tests/restrict-github-team-management/external-name-adoption/kyverno-test.yaml new file mode 100644 index 000000000..c228e43f7 --- /dev/null +++ b/tests/restrict-github-team-management/external-name-adoption/kyverno-test.yaml @@ -0,0 +1,31 @@ +--- +apiVersion: cli.kyverno.io/v1alpha1 +kind: Test +metadata: + name: restrict-github-team-management-external-name-adoption +policies: + - >- + ../../../k8s/bases/infrastructure/cluster-policies/best-practices/restrict-github-team-management.yaml +resources: + - resources.yaml +results: + # Both the object name and the spec identity are allow-listed and consistent, + # so neither earlier rule sees this. + - policy: restrict-github-team-management + rule: teams-allow-listed + resources: + - github-config/platform + kind: Team + result: pass + - policy: restrict-github-team-management + rule: teams-bind-provider-identity-to-object-name + resources: + - github-config/platform + kind: Team + result: pass + - policy: restrict-github-team-management + rule: teams-block-foreign-external-name-adoption + resources: + - github-config/platform + kind: Team + result: fail diff --git a/tests/restrict-github-team-management/external-name-adoption/resources.yaml b/tests/restrict-github-team-management/external-name-adoption/resources.yaml new file mode 100644 index 000000000..f42455219 --- /dev/null +++ b/tests/restrict-github-team-management/external-name-adoption/resources.yaml @@ -0,0 +1,14 @@ +--- +# Adoption of a pre-existing foreign GitHub team through the Crossplane +# external-name annotation, which bypasses both the object name and the spec +# fields. +apiVersion: team.github.m.upbound.io/v1beta1 +kind: Team +metadata: + name: platform + namespace: github-config + annotations: + crossplane.io/external-name: some-other-team +spec: + forProvider: + name: platform diff --git a/tests/restrict-github-team-management/forprovider-mismatch/kyverno-test.yaml b/tests/restrict-github-team-management/forprovider-mismatch/kyverno-test.yaml new file mode 100644 index 000000000..0a4727f33 --- /dev/null +++ b/tests/restrict-github-team-management/forprovider-mismatch/kyverno-test.yaml @@ -0,0 +1,25 @@ +--- +apiVersion: cli.kyverno.io/v1alpha1 +kind: Test +metadata: + name: restrict-github-team-management-forprovider-mismatch +policies: + - >- + ../../../k8s/bases/infrastructure/cluster-policies/best-practices/restrict-github-team-management.yaml +resources: + - resources.yaml +results: + # The object name is allow-listed, so that rule cannot catch this. + - policy: restrict-github-team-management + rule: teams-allow-listed + resources: + - github-config/platform + kind: Team + result: pass + # Only the identity binding catches it. + - policy: restrict-github-team-management + rule: teams-bind-provider-identity-to-object-name + resources: + - github-config/platform + kind: Team + result: fail diff --git a/tests/restrict-github-team-management/forprovider-mismatch/resources.yaml b/tests/restrict-github-team-management/forprovider-mismatch/resources.yaml new file mode 100644 index 000000000..668528e9f --- /dev/null +++ b/tests/restrict-github-team-management/forprovider-mismatch/resources.yaml @@ -0,0 +1,15 @@ +--- +# THE ESCALATION THIS RULE EXISTS TO STOP. metadata.name passes the allow-list, +# so a TeamMembership with teamIdRef.name: platform resolves to this object — +# while the org-admin provider reconciles the unrelated team named here. +# Its own fixture directory because it must reuse the allow-listed object name +# that the paved-road fixture already occupies. +apiVersion: team.github.m.upbound.io/v1beta1 +kind: Team +metadata: + name: platform + namespace: github-config +spec: + forProvider: + name: some-other-team + privacy: closed diff --git a/tests/restrict-github-team-management/initprovider-mismatch/kyverno-test.yaml b/tests/restrict-github-team-management/initprovider-mismatch/kyverno-test.yaml new file mode 100644 index 000000000..1f8d9c4a5 --- /dev/null +++ b/tests/restrict-github-team-management/initprovider-mismatch/kyverno-test.yaml @@ -0,0 +1,23 @@ +--- +apiVersion: cli.kyverno.io/v1alpha1 +kind: Test +metadata: + name: restrict-github-team-management-initprovider-mismatch +policies: + - >- + ../../../k8s/bases/infrastructure/cluster-policies/best-practices/restrict-github-team-management.yaml +resources: + - resources.yaml +results: + - policy: restrict-github-team-management + rule: teams-allow-listed + resources: + - github-config/platform + kind: Team + result: pass + - policy: restrict-github-team-management + rule: teams-bind-provider-identity-to-object-name + resources: + - github-config/platform + kind: Team + result: fail diff --git a/tests/restrict-github-team-management/initprovider-mismatch/resources.yaml b/tests/restrict-github-team-management/initprovider-mismatch/resources.yaml new file mode 100644 index 000000000..5db518e3e --- /dev/null +++ b/tests/restrict-github-team-management/initprovider-mismatch/resources.yaml @@ -0,0 +1,11 @@ +--- +# The same detachment expressed through initProvider, which upjet merges into +# forProvider when the field is unset there. +apiVersion: team.github.m.upbound.io/v1beta1 +kind: Team +metadata: + name: platform + namespace: github-config +spec: + initProvider: + name: some-other-team diff --git a/tests/restrict-github-team-management/kyverno-test.yaml b/tests/restrict-github-team-management/kyverno-test.yaml new file mode 100644 index 000000000..80c6206dd --- /dev/null +++ b/tests/restrict-github-team-management/kyverno-test.yaml @@ -0,0 +1,72 @@ +--- +apiVersion: cli.kyverno.io/v1alpha1 +kind: Test +metadata: + name: restrict-github-team-management +policies: + - >- + ../../k8s/bases/infrastructure/cluster-policies/best-practices/restrict-github-team-management.yaml +resources: + - resources.yaml +results: + # Only the CODEOWNERS teams may be managed through this delegation. + - policy: restrict-github-team-management + rule: teams-allow-listed + resources: + - github-config/attacker-team + kind: Team + result: fail + - policy: restrict-github-team-management + rule: teams-allow-listed + resources: + - github-config/platform + - github-config/maintainers + kind: Team + result: pass + # A self-consistent provider identity passes, including when it is omitted and + # Crossplane derives the remote team from the object name. attacker-team is + # here on purpose: it fails the allow-list above yet must pass this rule, so a + # regression that collapsed the two rules into one would be caught. + - policy: restrict-github-team-management + rule: teams-bind-provider-identity-to-object-name + resources: + - github-config/platform + - github-config/maintainers + - github-config/attacker-team + kind: Team + result: pass + # Creation without the annotation is the normal path. + - policy: restrict-github-team-management + rule: teams-block-foreign-external-name-adoption + resources: + - github-config/platform + - 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 + resources: + - github-config/direct-team-id + kind: TeamMembership + result: fail + - policy: restrict-github-team-management + rule: teammemberships-reference-allow-listed-teams + resources: + - github-config/platform-member + kind: TeamMembership + result: pass + # Repository grants are capped below admin. + - policy: restrict-github-team-management + rule: teamrepositories-reference-allow-listed-teams + resources: + - github-config/platform-repo-admin + kind: TeamRepository + result: fail + - policy: restrict-github-team-management + rule: teamrepositories-reference-allow-listed-teams + resources: + - github-config/platform-repo + kind: TeamRepository + result: pass diff --git a/tests/restrict-github-team-management/resources.yaml b/tests/restrict-github-team-management/resources.yaml new file mode 100644 index 000000000..fb9066ab8 --- /dev/null +++ b/tests/restrict-github-team-management/resources.yaml @@ -0,0 +1,87 @@ +--- +# The paved road: object name is allow-listed and the provider identity agrees +# with it, so the reference rules mean what they say. +apiVersion: team.github.m.upbound.io/v1beta1 +kind: Team +metadata: + name: platform + namespace: github-config +spec: + forProvider: + name: platform + privacy: closed +--- +# Allow-listed with the provider identity omitted entirely — Crossplane then +# derives the remote team from the object name, which is the shape the +# allow-list was written for. +apiVersion: team.github.m.upbound.io/v1beta1 +kind: Team +metadata: + name: maintainers + namespace: github-config +spec: + forProvider: + privacy: closed +--- +# A team outside the allow-list. Its provider identity is self-consistent, so it +# must fail the allow-list rule and pass the identity-binding rule — the two +# rules are independent. +apiVersion: team.github.m.upbound.io/v1beta1 +kind: Team +metadata: + name: attacker-team + namespace: github-config +spec: + forProvider: + name: attacker-team +--- +# A TeamMembership on the paved road. +apiVersion: team.github.m.upbound.io/v1beta1 +kind: TeamMembership +metadata: + name: platform-member + namespace: github-config +spec: + forProvider: + teamIdRef: + name: platform + username: devantler + role: member +--- +# A direct numeric team ID bypasses the reference allow-list entirely. +apiVersion: team.github.m.upbound.io/v1beta1 +kind: TeamMembership +metadata: + name: direct-team-id + namespace: github-config +spec: + forProvider: + teamId: "1234567" + username: attacker + role: maintainer +--- +# A TeamRepository on the paved road. +apiVersion: team.github.m.upbound.io/v1beta1 +kind: TeamRepository +metadata: + name: platform-repo + namespace: github-config +spec: + forProvider: + teamIdRef: + name: platform + repository: platform + permission: push +--- +# Admin is the privileged repository grant the rule blocks. +apiVersion: team.github.m.upbound.io/v1beta1 +kind: TeamRepository +metadata: + name: platform-repo-admin + namespace: github-config +spec: + forProvider: + teamIdRef: + name: platform + repository: platform + permission: admin From e1bd00251fd5fd6c6609ecd4b6652a3d7400a42d Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sat, 15 Aug 2026 09:09:53 +0200 Subject: [PATCH 3/5] fix(cluster-policies): scope the external-name rule with match operations The rule gated CREATE with a {{ request.operation }} precondition, but the policy sets background: true and a background scan has no AdmissionReview to read request.* from. Expressing the scope as a match operation keeps it background-safe and simply does not select during a background scan. --- .../restrict-github-team-management.yaml | 12 +++++++----- 1 file changed, 7 insertions(+), 5 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 e06e45d35..1ee1c5a5d 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 @@ -87,6 +87,11 @@ spec: # UPDATE would block the provider's own reconciliation. Tenant-authored # UPDATE of this annotation is therefore still open and needs the provider # ServiceAccount excluded to close; tracked separately. + # The CREATE scope is expressed as a match operation rather than a + # {{ request.operation }} precondition on purpose: this policy sets + # background: true, and a background scan has no AdmissionReview to read + # request.* from. Matching on operations keeps the rule background-safe and + # simply does not select during a background scan. - name: teams-block-foreign-external-name-adoption match: any: @@ -95,11 +100,8 @@ spec: - team.github.m.upbound.io/*/Team namespaces: - github-config - preconditions: - all: - - key: "{{ request.operation || 'BACKGROUND' }}" - operator: Equals - value: CREATE + operations: + - CREATE validate: message: GitHub Team resources must not set crossplane.io/external-name to a different team at creation; omit it or set it to metadata.name. deny: From 5186097f57593ec56b95b713892d6a9b4b08eded Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sat, 15 Aug 2026 09:19:26 +0200 Subject: [PATCH 4/5] fix(cluster-policies): close the initProvider bypass; drop the external-name rule Addresses the Codex review at 2b57d419. upjet merges initProvider into unset forProvider fields, so the reference rules inspecting only forProvider could be fronted: an approved forProvider teamIdRef alongside a foreign initProvider teamId, or permission: push alongside initProvider permission: admin. Both reference rules now apply every ID, selector, reference and permission constraint to initProvider as well. Removes teams-block-foreign-external-name-adoption. It looked like the natural counterpart to the identity binding but broke legitimate re-adoption: after a rebuild the existing GitHub team must be adopted by the numeric team ID the provider itself writes to that annotation, and accepting only the object name would make Crossplane try to create a team that already exists, so github-config could not recover its own teams. Constraining it safely needs a way to tell our teams' numeric identities from a foreign one plus the provider ServiceAccount excluded; folded into #3144 rather than shipped half-closed. --- .../restrict-github-team-management.yaml | 81 +++++++++++-------- .../external-name-adoption/kyverno-test.yaml | 31 ------- .../external-name-adoption/resources.yaml | 14 ---- .../kyverno-test.yaml | 11 +-- .../resources.yaml | 31 +++++++ 5 files changed, 80 insertions(+), 88 deletions(-) delete mode 100644 tests/restrict-github-team-management/external-name-adoption/kyverno-test.yaml delete mode 100644 tests/restrict-github-team-management/external-name-adoption/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 1ee1c5a5d..48e838cd9 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 @@ -80,38 +80,17 @@ spec: - "" - "{{ request.object.metadata.name }}" # crossplane.io/external-name is the other way the local name can be - # detached from the remote team: setting it at creation adopts a - # pre-existing GitHub team regardless of what metadata.name says. This is - # scoped to CREATE deliberately — provider-upjet-github writes the numeric - # team ID back into this annotation after creation, so enforcing it on - # UPDATE would block the provider's own reconciliation. Tenant-authored - # UPDATE of this annotation is therefore still open and needs the provider - # ServiceAccount excluded to close; tracked separately. - # The CREATE scope is expressed as a match operation rather than a - # {{ request.operation }} precondition on purpose: this policy sets - # background: true, and a background scan has no AdmissionReview to read - # request.* from. Matching on operations keeps the rule background-safe and - # simply does not select during a background scan. - - name: teams-block-foreign-external-name-adoption - match: - any: - - resources: - kinds: - - team.github.m.upbound.io/*/Team - namespaces: - - github-config - operations: - - CREATE - validate: - message: GitHub Team resources must not set crossplane.io/external-name to a different team at creation; omit it or set it to metadata.name. - deny: - conditions: - any: - - key: "{{ request.object.metadata.annotations.\"crossplane.io/external-name\" || '' }}" - operator: AnyNotIn - value: - - "" - - "{{ request.object.metadata.name }}" + # detached from the remote team, and it is NOT constrained here. A rule + # accepting only an empty annotation or metadata.name looks like the + # obvious counterpart to the binding above, but it breaks legitimate + # re-adoption: after a rebuild, or whenever a Team CR is recreated, the + # existing GitHub team must be adopted by the numeric team ID the provider + # itself writes into this annotation — and refusing that makes Crossplane + # try to create a team that already exists, so github-config cannot recover + # its own teams. Constraining it safely needs a way to tell our teams' + # 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. - name: teammemberships-reference-allow-listed-teams match: any: @@ -121,7 +100,7 @@ spec: namespaces: - github-config validate: - message: GitHub TeamMembership resources must use spec.forProvider.teamIdRef.name set to platform or maintainers; direct teamId values and selectors are not allowed. + message: GitHub TeamMembership resources must use spec.forProvider.teamIdRef.name set to platform or maintainers; direct teamId values and selectors are not allowed, in forProvider or initProvider. deny: conditions: any: @@ -136,6 +115,22 @@ spec: value: - platform - maintainers + # upjet merges initProvider into any unset forProvider field, so + # every constraint above has to hold there too — otherwise an + # approved forProvider reference fronts a foreign team supplied + # under initProvider. + - key: "{{ request.object.spec.initProvider.teamId || '' }}" + operator: NotEquals + value: "" + - key: "{{ request.object.spec.initProvider.teamIdSelector || `{}` }}" + operator: NotEquals + value: {} + - key: "{{ request.object.spec.initProvider.teamIdRef.name || '' }}" + operator: AnyNotIn + value: + - "" + - platform + - maintainers - name: teamrepositories-reference-allow-listed-teams match: any: @@ -145,7 +140,7 @@ spec: namespaces: - github-config validate: - message: GitHub TeamRepository resources must reference platform or maintainers by spec.forProvider.teamIdRef.name and may not grant admin permission. + message: GitHub TeamRepository resources must reference platform or maintainers by spec.forProvider.teamIdRef.name and may not grant admin permission, in forProvider or initProvider. deny: conditions: any: @@ -163,3 +158,21 @@ spec: - key: "{{ request.object.spec.forProvider.permission || '' }}" operator: Equals value: admin + # Same initProvider merge as above: an approved forProvider + # reference must not be able to front a foreign team or an admin + # grant supplied under initProvider. + - key: "{{ request.object.spec.initProvider.teamId || '' }}" + operator: NotEquals + value: "" + - key: "{{ request.object.spec.initProvider.teamIdSelector || `{}` }}" + operator: NotEquals + value: {} + - key: "{{ request.object.spec.initProvider.teamIdRef.name || '' }}" + operator: AnyNotIn + value: + - "" + - platform + - maintainers + - key: "{{ request.object.spec.initProvider.permission || '' }}" + operator: Equals + value: admin diff --git a/tests/restrict-github-team-management/external-name-adoption/kyverno-test.yaml b/tests/restrict-github-team-management/external-name-adoption/kyverno-test.yaml deleted file mode 100644 index c228e43f7..000000000 --- a/tests/restrict-github-team-management/external-name-adoption/kyverno-test.yaml +++ /dev/null @@ -1,31 +0,0 @@ ---- -apiVersion: cli.kyverno.io/v1alpha1 -kind: Test -metadata: - name: restrict-github-team-management-external-name-adoption -policies: - - >- - ../../../k8s/bases/infrastructure/cluster-policies/best-practices/restrict-github-team-management.yaml -resources: - - resources.yaml -results: - # Both the object name and the spec identity are allow-listed and consistent, - # so neither earlier rule sees this. - - policy: restrict-github-team-management - rule: teams-allow-listed - resources: - - github-config/platform - kind: Team - result: pass - - policy: restrict-github-team-management - rule: teams-bind-provider-identity-to-object-name - resources: - - github-config/platform - kind: Team - result: pass - - policy: restrict-github-team-management - rule: teams-block-foreign-external-name-adoption - resources: - - github-config/platform - kind: Team - result: fail diff --git a/tests/restrict-github-team-management/external-name-adoption/resources.yaml b/tests/restrict-github-team-management/external-name-adoption/resources.yaml deleted file mode 100644 index f42455219..000000000 --- a/tests/restrict-github-team-management/external-name-adoption/resources.yaml +++ /dev/null @@ -1,14 +0,0 @@ ---- -# Adoption of a pre-existing foreign GitHub team through the Crossplane -# external-name annotation, which bypasses both the object name and the spec -# fields. -apiVersion: team.github.m.upbound.io/v1beta1 -kind: Team -metadata: - name: platform - namespace: github-config - annotations: - crossplane.io/external-name: some-other-team -spec: - forProvider: - name: platform diff --git a/tests/restrict-github-team-management/kyverno-test.yaml b/tests/restrict-github-team-management/kyverno-test.yaml index 80c6206dd..62b07253a 100644 --- a/tests/restrict-github-team-management/kyverno-test.yaml +++ b/tests/restrict-github-team-management/kyverno-test.yaml @@ -35,20 +35,12 @@ results: - github-config/attacker-team kind: Team result: pass - # Creation without the annotation is the normal path. - - policy: restrict-github-team-management - rule: teams-block-foreign-external-name-adoption - resources: - - github-config/platform - - 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 resources: - github-config/direct-team-id + - github-config/initprovider-foreign-team kind: TeamMembership result: fail - policy: restrict-github-team-management @@ -62,6 +54,7 @@ results: rule: teamrepositories-reference-allow-listed-teams resources: - github-config/platform-repo-admin + - github-config/initprovider-admin-grant kind: TeamRepository result: fail - policy: restrict-github-team-management diff --git a/tests/restrict-github-team-management/resources.yaml b/tests/restrict-github-team-management/resources.yaml index fb9066ab8..04f806733 100644 --- a/tests/restrict-github-team-management/resources.yaml +++ b/tests/restrict-github-team-management/resources.yaml @@ -85,3 +85,34 @@ spec: name: platform repository: platform permission: admin +--- +# initProvider bypass: the forProvider reference is approved, but upjet merges +# initProvider into unset forProvider fields, so a foreign team supplied there +# is what actually reconciles. +apiVersion: team.github.m.upbound.io/v1beta1 +kind: TeamMembership +metadata: + name: initprovider-foreign-team + namespace: github-config +spec: + forProvider: + teamIdRef: + name: platform + username: attacker + initProvider: + teamId: "7654321" +--- +# Same bypass for the admin grant the TeamRepository rule blocks. +apiVersion: team.github.m.upbound.io/v1beta1 +kind: TeamRepository +metadata: + name: initprovider-admin-grant + namespace: github-config +spec: + forProvider: + teamIdRef: + name: platform + repository: platform + permission: push + initProvider: + permission: admin From 426766d469cb67e56f800595bdfb1841a818a336 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Sat, 15 Aug 2026 09:38:24 +0200 Subject: [PATCH 5/5] fix(cluster-policies): allow-list team repository permissions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the CodeRabbit review at 5186097f. The rule denied the exact value admin, but GitHub also offers maintain and organization-defined custom repository roles, so the guard failed open the moment the tenant selected any other privileged role. Replaced with an allow-list of pull/triage/push (empty being the provider default), applied to forProvider and initProvider alike — matching the allow-list style the rest of the policy already uses. Also covers three conditions no fixture exercised: teamIdSelector, a by-name reference to a non-allow-listed team, and the maintain grant above. The selector condition compares a JMESPath map literal against {}, which is exactly the kind of thing that silently stops matching. --- .../restrict-github-team-management.yaml | 23 ++++++++--- .../kyverno-test.yaml | 3 ++ .../resources.yaml | 41 +++++++++++++++++++ 3 files changed, 62 insertions(+), 5 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 48e838cd9..8cf35a405 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 @@ -140,7 +140,7 @@ spec: namespaces: - github-config validate: - message: GitHub TeamRepository resources must reference platform or maintainers by spec.forProvider.teamIdRef.name and may not grant admin permission, in forProvider or initProvider. + message: GitHub TeamRepository resources must reference platform or maintainers by spec.forProvider.teamIdRef.name and may grant only pull, triage or push permission, in forProvider and initProvider. deny: conditions: any: @@ -155,9 +155,18 @@ spec: value: - platform - maintainers + # An allow-list, not a deny-list on `admin`: GitHub also offers + # `maintain` and organization-defined custom repository roles, so + # denying only `admin` fails open the moment the tenant picks + # another privileged role. An empty value is the provider default + # (`pull`). - key: "{{ request.object.spec.forProvider.permission || '' }}" - operator: Equals - value: admin + operator: AnyNotIn + value: + - "" + - pull + - triage + - push # Same initProvider merge as above: an approved forProvider # reference must not be able to front a foreign team or an admin # grant supplied under initProvider. @@ -174,5 +183,9 @@ spec: - platform - maintainers - key: "{{ request.object.spec.initProvider.permission || '' }}" - operator: Equals - value: admin + operator: AnyNotIn + value: + - "" + - pull + - triage + - push diff --git a/tests/restrict-github-team-management/kyverno-test.yaml b/tests/restrict-github-team-management/kyverno-test.yaml index 62b07253a..861399ffa 100644 --- a/tests/restrict-github-team-management/kyverno-test.yaml +++ b/tests/restrict-github-team-management/kyverno-test.yaml @@ -41,6 +41,8 @@ results: resources: - github-config/direct-team-id - github-config/initprovider-foreign-team + - github-config/selector-reference + - github-config/non-allow-listed-reference kind: TeamMembership result: fail - policy: restrict-github-team-management @@ -55,6 +57,7 @@ results: resources: - github-config/platform-repo-admin - github-config/initprovider-admin-grant + - github-config/maintain-grant kind: TeamRepository result: fail - policy: restrict-github-team-management diff --git a/tests/restrict-github-team-management/resources.yaml b/tests/restrict-github-team-management/resources.yaml index 04f806733..fc2b94979 100644 --- a/tests/restrict-github-team-management/resources.yaml +++ b/tests/restrict-github-team-management/resources.yaml @@ -116,3 +116,44 @@ spec: permission: push initProvider: permission: admin +--- +# teamIdSelector: a label selector resolves to whatever Team matches, so it +# bypasses the by-name allow-list entirely. Exercises the JMESPath map-literal +# comparison against `{}`, which nothing else covers. +apiVersion: team.github.m.upbound.io/v1beta1 +kind: TeamMembership +metadata: + name: selector-reference + namespace: github-config +spec: + forProvider: + teamIdSelector: + matchLabels: + role: privileged + username: attacker +--- +# A by-name reference that resolves, but to a team outside the allow-list. +apiVersion: team.github.m.upbound.io/v1beta1 +kind: TeamMembership +metadata: + name: non-allow-listed-reference + namespace: github-config +spec: + forProvider: + teamIdRef: + name: attacker-team + username: attacker +--- +# maintain is privileged but is not `admin`, so a deny-list on admin alone lets +# it through. This is the fail-open the allow-list closes. +apiVersion: team.github.m.upbound.io/v1beta1 +kind: TeamRepository +metadata: + name: maintain-grant + namespace: github-config +spec: + forProvider: + teamIdRef: + name: platform + repository: platform + permission: maintain