Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
7 changes: 4 additions & 3 deletions k8s/bases/apps/github-config/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,10 @@ rules:
- 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.
# the restrict-github-team-management and
# restrict-github-team-external-identity Kyverno admission policies constrain
# 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,119 @@
# Pins which GitHub team a github-config Team object reconciles, for the whole
# life of the object (#3144).
#
# restrict-github-team-management binds spec.forProvider.name to the object
# name, but the provider finds the remote team by the numeric ID in the
# crossplane.io/external-name annotation, not by name. Whoever can write that
# annotation can point the allow-listed object `admins` at any team in the
# organization, and every TeamMembership and TeamRepository that references
# `admins` by name then acts on that team through the org-admin provider.
#
# Measured on prod (2026-10-05), the annotation has two legitimate writers:
#
# - the provider itself, which records the ID of a team it has just created
# (`admins`: the annotation's field owner is the provider);
# - the github-config artifact, which adopts a team that already exists by
# stating its ID (`maintainers`: Observe-only, field owner is the applier).
# The same path recovers both teams after a rebuild.
#
# So neither "never set it" nor "only the provider sets it" holds. The rule
# below accepts, from anyone but the provider, exactly three cases: no
# annotation, the value the object already carries, or the approved identity of
# that object name. Everything else is a foreign team and is refused on create
# and on update alike.
#
# The approved identities live HERE and are not read from the github-config
# artifact, for the reason the member allow-list gives: that artifact is the
# input this policy constrains. A team that is deleted and created again gets a
# new ID; the provider's own write of it is admitted, the object keeps working
# because an unchanged value is always accepted, and the entry below is then
# updated in a reviewed change so the new ID can be used for recovery.
#
# The provider is exempt because the ID of a team it has just created cannot be
# known in advance. Its ServiceAccount is named after the provider package
# revision (provider-upjet-github-<revision>, read from the running pod), so
# the name changes with every provider upgrade and is matched by prefix.
# Creating a ServiceAccount in crossplane-system is a platform-level
# permission, so the prefix cannot be claimed from a tenant namespace. It would
# also cover another package whose name starts the same way, should one ever
# be installed there.
#
# Admission-only (background: false), because the exemption is on the writing
# identity, which a background scan does not have.
apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
name: restrict-github-team-external-identity
annotations:
policies.kyverno.io/title: Restrict GitHub Team External Identity
policies.kyverno.io/category: Security, GitHub
policies.kyverno.io/severity: high
policies.kyverno.io/subject: Team
policies.kyverno.io/minversion: 1.13.0
policies.kyverno.io/description: >-
Refuses a github-config Team whose crossplane.io/external-name is set or
changed to anything other than the approved numeric identity of that team,
so an allow-listed Team object cannot be pointed at a different GitHub
team after it was admitted. The provider's own record of a team it
created is exempt.
spec:
background: false
rules:
- name: teams-external-name-is-an-approved-identity
match:
any:
- resources:
kinds:
- team.github.m.upbound.io/*/Team
operations:
- CREATE
- UPDATE
namespaces:
- github-config
exclude:
any:
- subjects:
- kind: ServiceAccount
name: provider-upjet-github-*
namespace: crossplane-system
context:
- name: externalName
variable:
jmesPath: request.object.metadata.annotations."crossplane.io/external-name"
default: ""
- name: previousExternalName
variable:
jmesPath: request.oldObject.metadata.annotations."crossplane.io/external-name"
default: ""
validate:
failureAction: Enforce
# Kyverno admits an update by default when the object ALREADY failed the
# rule before it. Here that would be a way round the rule: a team the
# provider created anew carries an identity that is not approved yet, so
# its object "already fails" when judged alone, and every later change
# to its identity would be waved through. An unchanged value is accepted
# by the condition below instead, which is the only tolerance needed.
allowExistingViolations: false
message: >-
The crossplane.io/external-name of a github-config Team may only be
left unset, left unchanged, or set to the approved numeric identity of
that team. Adopting a team under a new identity is a reviewed change
to restrict-github-team-external-identity.yaml.
deny:
conditions:
all:
# ONE exact comparison, evaluated by JMESPath, and deliberately
# not three Kyverno operators. AnyNotIn reads * and ? as wildcards
# in either operand, Equals reads them in its value and compares
# numbers as quantities, and the annotation is written by the party
# this rule constrains: an identity of "*" would match the approved
# one, and any later value would then count as "unchanged" from
# "*". JMESPath == and contains() compare strings literally.
#
# Accepted: no identity, the identity the object already has, or
# the approved identity of this object name. Name and identity
# are checked as a pair, so the approved identity of one team
# cannot be used on the other object.
- key: "{{ externalName == '' || externalName == previousExternalName || contains(['admins/18573307', 'maintainers/17046606'], join('/', [request.object.metadata.name, externalName])) }}"
operator: Equals
value: false
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,11 @@ spec:
# 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.
# itself writes into this annotation. It is constrained in
# restrict-github-team-external-identity instead, which approves each
# 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.
- name: teammemberships-reference-allow-listed-teams
match:
any:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,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-external-identity.yaml
- best-practices/restrict-github-team-management.yaml
- best-practices/restrict-homepage-service-groups.yaml
- best-practices/restrict-tenant-issuer-refs.yaml
Expand Down
1 change: 1 addition & 0 deletions tests/policy-failure-actions.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"audit-privileged-rbac/privileged-rules": "Enforce",
"disallow-latest-tag/require-image-tag": "Audit",
"disallow-latest-tag/validate-image-tag": "Audit",
"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/teammemberships-reference-allow-listed-teams": "Enforce",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
apiVersion: cli.kyverno.io/v1alpha1
kind: Test
metadata:
name: restrict-github-team-external-identity-crossplane-identity
policies:
- >-
../../../k8s/bases/infrastructure/cluster-policies/best-practices/restrict-github-team-external-identity.yaml
resources:
- resources.yaml
variables: values.yaml
userinfo: user-info.yaml
results:
- policy: restrict-github-team-external-identity
rule: teams-external-name-is-an-approved-identity
resources:
- github-config/admins
kind: Team
result: fail
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
# Only the GitHub provider is exempt, not every ServiceAccount beside it: Crossplane itself and the other providers are held to the rule.
apiVersion: team.github.m.upbound.io/v1beta1
kind: Team
metadata:
name: admins
namespace: github-config
annotations:
crossplane.io/external-name: "99999999"
spec:
forProvider:
privacy: closed
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
apiVersion: cli.kyverno.io/v1alpha1
kind: UserInfo
metadata:
name: crossplane-core
userInfo:
username: system:serviceaccount:crossplane-system:crossplane
groups:
- system:serviceaccounts
- system:serviceaccounts:crossplane-system
- system:authenticated
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
apiVersion: cli.kyverno.io/v1alpha1
kind: Values
metadata:
name: values
globalValues:
request.operation: UPDATE
# kyverno test presents an update with the previous object equal to the new
# one. The values below are merged into the previous object, so the row tests a
# change.
policies:
- name: restrict-github-team-external-identity
resources:
- name: admins
values:
request.oldObject:
metadata:
name: admins
namespace: github-config
annotations:
crossplane.io/external-name: "18573307"
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
apiVersion: cli.kyverno.io/v1alpha1
kind: Test
metadata:
name: restrict-github-team-external-identity-foreign
policies:
- >-
../../../k8s/bases/infrastructure/cluster-policies/best-practices/restrict-github-team-external-identity.yaml
resources:
- resources.yaml
variables: values.yaml
userinfo: user-info.yaml
results:
- policy: restrict-github-team-external-identity
rule: teams-external-name-is-an-approved-identity
resources:
- github-config/admins
- github-config/maintainers
- github-config/other
kind: Team
result: fail
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
---
# The escalation: an allow-listed name created against a foreign team.
apiVersion: team.github.m.upbound.io/v1beta1
kind: Team
metadata:
name: admins
namespace: github-config
annotations:
crossplane.io/external-name: "99999999"
spec:
forProvider:
privacy: closed
---
# A slug is not an approved identity either, whatever name it spells.
apiVersion: team.github.m.upbound.io/v1beta1
kind: Team
metadata:
name: maintainers
namespace: github-config
annotations:
crossplane.io/external-name: "admins"
spec:
forProvider:
privacy: closed
---
# An approved identity under a name it does not belong to (the name itself is refused by restrict-github-team-management).
apiVersion: team.github.m.upbound.io/v1beta1
kind: Team
metadata:
name: other
namespace: github-config
annotations:
crossplane.io/external-name: "17046606"
spec:
forProvider:
privacy: closed
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
apiVersion: cli.kyverno.io/v1alpha1
kind: UserInfo
metadata:
name: github-config-source
userInfo:
username: system:serviceaccount:github-config:github-config
groups:
- system:serviceaccounts
- system:serviceaccounts:github-config
- system:authenticated
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
apiVersion: cli.kyverno.io/v1alpha1
kind: Values
metadata:
name: values
globalValues:
request.operation: CREATE
20 changes: 20 additions & 0 deletions tests/restrict-github-team-external-identity/kyverno-test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
apiVersion: cli.kyverno.io/v1alpha1
kind: Test
metadata:
name: restrict-github-team-external-identity
policies:
- >-
../../k8s/bases/infrastructure/cluster-policies/best-practices/restrict-github-team-external-identity.yaml
resources:
- resources.yaml
variables: values.yaml
userinfo: user-info.yaml
results:
- policy: restrict-github-team-external-identity
rule: teams-external-name-is-an-approved-identity
resources:
- github-config/admins
- github-config/maintainers
kind: Team
result: pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
apiVersion: cli.kyverno.io/v1alpha1
kind: Test
metadata:
name: restrict-github-team-external-identity-lookalike-identity
policies:
- >-
../../../k8s/bases/infrastructure/cluster-policies/best-practices/restrict-github-team-external-identity.yaml
resources:
- resources.yaml
variables: values.yaml
userinfo: user-info.yaml
results:
- policy: restrict-github-team-external-identity
rule: teams-external-name-is-an-approved-identity
resources:
- github-config/admins
kind: Team
result: fail
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
# A ServiceAccount with the provider's name in a tenant namespace is not the provider.
apiVersion: team.github.m.upbound.io/v1beta1
kind: Team
metadata:
name: admins
namespace: github-config
annotations:
crossplane.io/external-name: "99999999"
spec:
forProvider:
privacy: closed
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
apiVersion: cli.kyverno.io/v1alpha1
kind: UserInfo
metadata:
name: lookalike
userInfo:
username: system:serviceaccount:github-config:provider-upjet-github-f5d83a3e04ea
groups:
- system:serviceaccounts
- system:serviceaccounts:github-config
- system:authenticated
Loading
Loading