Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions k8s/bases/apps/github-config/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ rules:
- update
- patch
- delete
# Team resources are high-impact because they change real GitHub org
# authorization. RBAC alone cannot express which teams this SA may manage, so
# the restrict-github-team-management Kyverno admission policy constrains the
# allowed team identities, reference style, and repository permissions on top
# of these verbs.
- apiGroups:
- team.github.m.upbound.io
resources:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
# 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
Comment thread
coderabbitai[bot] marked this conversation as resolved.
background: true
rules:
- name: teams-allow-listed
match:
any:
- resources:
kinds:
- team.github.m.upbound.io/*/Team
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 }}"
Comment thread
devantler marked this conversation as resolved.
Comment thread
devantler marked this conversation as resolved.
operator: AnyNotIn
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 }}"
Comment thread
devantler marked this conversation as resolved.
Outdated
- name: teammemberships-reference-allow-listed-teams
match:
any:
- resources:
kinds:
- team.github.m.upbound.io/*/TeamMembership
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: ""
Comment thread
devantler marked this conversation as resolved.
Outdated
- key: "{{ request.object.spec.forProvider.teamIdSelector || `{}` }}"
operator: NotEquals
value: {}
- key: "{{ request.object.spec.forProvider.teamIdRef.name || '' }}"
operator: AnyNotIn
value:
- platform
- maintainers
Comment thread
devantler marked this conversation as resolved.
- name: teamrepositories-reference-allow-listed-teams
match:
any:
- resources:
kinds:
- team.github.m.upbound.io/*/TeamRepository
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 || '' }}"
Comment thread
devantler marked this conversation as resolved.
operator: Equals
value: admin
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
Original file line number Diff line number Diff line change
Expand Up @@ -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-issuer-refs.yaml
- best-practices/restrict-tenant-secret-stores.yaml
- best-practices/validate-host-restrictions.yaml
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
72 changes: 72 additions & 0 deletions tests/restrict-github-team-management/kyverno-test.yaml
Original file line number Diff line number Diff line change
@@ -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.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
- 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
Loading
Loading