Script 'mail_helper' called by obssrc Hello community, here is the log from the commit of package kargo-cli for openSUSE:Factory checked in at 2026-07-01 16:51:20 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Comparing /work/SRC/openSUSE:Factory/kargo-cli (Old) and /work/SRC/openSUSE:Factory/.kargo-cli.new.11887 (New) ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Package is "kargo-cli" Wed Jul 1 16:51:20 2026 rev:56 rq:1362883 version:1.10.8 Changes: -------- --- /work/SRC/openSUSE:Factory/kargo-cli/kargo-cli.changes 2026-06-13 18:50:36.281573634 +0200 +++ /work/SRC/openSUSE:Factory/.kargo-cli.new.11887/kargo-cli.changes 2026-07-01 16:52:09.761419867 +0200 @@ -1,0 +2,6 @@ +Wed Jul 01 07:07:44 UTC 2026 - Johannes Kastl <[email protected]> + +- Update to version 1.10.8: + no CLI-related changes or dependency updates + +------------------------------------------------------------------- Old: ---- kargo-cli-1.10.7.obscpio New: ---- kargo-cli-1.10.8.obscpio ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Other differences: ------------------ ++++++ kargo-cli.spec ++++++ --- /var/tmp/diff_new_pack.KAIf8x/_old 2026-07-01 16:52:16.165641711 +0200 +++ /var/tmp/diff_new_pack.KAIf8x/_new 2026-07-01 16:52:16.165641711 +0200 @@ -19,7 +19,7 @@ %define executable_name kargo Name: kargo-cli -Version: 1.10.7 +Version: 1.10.8 Release: 0 Summary: CLI for the Kubernetes Application lifecycle orchestration License: Apache-2.0 ++++++ _service ++++++ --- /var/tmp/diff_new_pack.KAIf8x/_old 2026-07-01 16:52:16.249644621 +0200 +++ /var/tmp/diff_new_pack.KAIf8x/_new 2026-07-01 16:52:16.293646145 +0200 @@ -1,9 +1,9 @@ <services> <service name="obs_scm" mode="manual"> - <param name="url">https://github.com/akuity/kargo</param> + <param name="url">https://github.com/akuity/kargo.git</param> <param name="scm">git</param> <param name="exclude">.git</param> - <param name="revision">v1.10.7</param> + <param name="revision">refs/tags/v1.10.8</param> <param name="versionformat">@PARENT_TAG@</param> <param name="versionrewrite-pattern">v(.*)</param> <param name="changesgenerate">enable</param> ++++++ _servicedata ++++++ --- /var/tmp/diff_new_pack.KAIf8x/_old 2026-07-01 16:52:16.541654736 +0200 +++ /var/tmp/diff_new_pack.KAIf8x/_new 2026-07-01 16:52:16.589656399 +0200 @@ -1,6 +1,8 @@ <servicedata> <service name="tar_scm"> <param name="url">https://github.com/akuity/kargo</param> - <param name="changesrevision">17077e406ed309fcc830d08776d9b9a47fe6e134</param></service></servicedata> + <param name="changesrevision">17077e406ed309fcc830d08776d9b9a47fe6e134</param></service><service name="tar_scm"> + <param name="url">https://github.com/akuity/kargo.git</param> + <param name="changesrevision">50d29c10153965114f903645c83eadbe7bf58d3f</param></service></servicedata> (No newline at EOF) ++++++ kargo-cli-1.10.7.obscpio -> kargo-cli-1.10.8.obscpio ++++++ diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kargo-cli-1.10.7/docs/docs/80-release-notes/89-v1.10.0.md new/kargo-cli-1.10.8/docs/docs/80-release-notes/89-v1.10.0.md --- old/kargo-cli-1.10.7/docs/docs/80-release-notes/89-v1.10.0.md 2026-06-12 02:30:12.000000000 +0200 +++ new/kargo-cli-1.10.8/docs/docs/80-release-notes/89-v1.10.0.md 2026-06-26 01:39:10.000000000 +0200 @@ -12,6 +12,13 @@ (`freightMetadata(freightName).keyName`) or map access syntax (`freightMetadata(freightName)['key-name']`) to access specific values +* **Promote Endpoint Response Shape** ([#6135](https://github.com/akuity/kargo/pull/6135)): The + `POST /v1beta1/projects/{project}/stages/{stage}/promotions` REST endpoint now returns a bare + `Promotion` object, as its OpenAPI spec has always documented. Prior to v1.10.0, the server + incorrectly wrapped the object as `{"promotion": {...}}`, so reality did not match the spec. The + server and CLI were corrected to match. As a result, a v1.10.0+ CLI is incompatible with a pre-v1.10.0 + server (and vice versa) for the `promote` command -- upgrade your CLI to match the server version. + ## ⚠️ New Deprecations {#new-deprecations} * **`git-push` Default Integration Policy Changing in v1.12.0**: The `git-push` step now supports diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kargo-cli-1.10.7/hack/cve-scan.sh new/kargo-cli-1.10.8/hack/cve-scan.sh --- old/kargo-cli-1.10.7/hack/cve-scan.sh 1970-01-01 01:00:00.000000000 +0100 +++ new/kargo-cli-1.10.8/hack/cve-scan.sh 2026-06-26 01:39:10.000000000 +0200 @@ -0,0 +1,198 @@ +#!/usr/bin/env bash +# +# Scan a container image for CVEs with Grype, applying Akuity's published VEX +# (Vulnerability Exploitability eXchange) statements so that findings already +# assessed as not-affected are suppressed. +# +# Report-only by default: findings are written to the job log and the GitHub +# step summary, and a warning annotation is emitted when Critical/High CVEs +# are present. Set FAIL_ON to a severity (e.g. "high") to make the scan fail +# the build instead. +# +# This is the canonical scan recipe used across Akuity image pipelines. +# +# Usage: cve-scan.sh <image-ref> [<vex-repo>] +# +# <image-ref> Image to scan, with an explicit grype scheme, e.g.: +# docker:us-docker.pkg.dev/akuity/akp/akuity-platform:1.2.3 +# (image in the local docker daemon, typical for PR builds) +# registry:us-docker.pkg.dev/akuity/akp/akuity-platform@sha256:... +# (image in a remote registry, typical for pushed images; +# prefer scanning by digest) +# <vex-repo> Repository path used to look up VEX statements at +# ${VEX_BASE_URL}/pkg/oci/<vex-repo>/vex.json. Defaults to the +# repository derived from <image-ref> (scheme/tag/digest +# stripped). A missing VEX document is not an error: VEX +# publishing may not yet cover this image. +# +# Environment: +# GRYPE_CMD Path to the grype binary (default: "grype" on PATH) +# FAIL_ON Severity threshold to fail on ("critical", "high", ...). +# Empty (default) = report only, never fail. +# VEX_BASE_URL Base URL of the VEX repository (default: https://vex.akuity.io) + +set -euo pipefail + +if [[ $# -lt 1 ]]; then + echo "usage: $0 <image-ref> [<vex-repo>]" >&2 + exit 2 +fi + +IMAGE_REF="$1" +GRYPE_CMD="${GRYPE_CMD:-grype}" +FAIL_ON="${FAIL_ON:-}" +VEX_BASE_URL="${VEX_BASE_URL:-https://vex.akuity.io}"; + +# Derive the repository from the image ref: strip the grype scheme prefix, +# any digest, and any tag (a colon after the last slash). +repo="${IMAGE_REF#docker:}" +repo="${repo#registry:}" +repo="${repo#oci-archive:}" +repo="${repo#docker-archive:}" +repo="${repo%%@*}" +base="${repo##*/}" +tag="" +if [[ "$base" == *:* ]]; then + tag="${base##*:}" + repo="${repo%:*}" +fi +VEX_REPO="${2:-$repo}" + +workdir="$(mktemp -d)" +trap 'rm -rf "$workdir"' EXIT + +# grype matches a VEX product @id only against the image's repo digest +# (<repo>@sha256:...) -- never the bare repo or <repo>:<tag>. The published doc +# carries the logical repo @id (it can't know each patch's digest), so we must +# re-stamp it to the digest of the image we are about to scan before applying. +# We stamp to the SCANNED image's repo@digest (not VEX_REPO): the statements +# come from VEX_REPO's published doc, but must carry the scanned image's +# identity to match. The package-level suppression is driven by the version- +# pinned subcomponent purls, which we leave untouched. +resolve_stamp_ref() { + # Already digest-pinned: reuse the digest directly. + if [[ "$IMAGE_REF" == *@sha256:* ]]; then + printf '%s@%s\n' "$repo" "${IMAGE_REF##*@}" + return + fi + # Tag ref -> resolve the digest grype will compute for this image. + case "$IMAGE_REF" in + docker:*) + # Local daemon: first RepoDigest (empty for --load builds never + # pushed -> no digest to match, so VEX is skipped below). + docker inspect --format \ + '{{if .RepoDigests}}{{index .RepoDigests 0}}{{end}}' \ + "${IMAGE_REF#docker:}" 2>/dev/null || true + ;; + registry:*) + # Remote / job-local registry tag: ask for the digest via crane. + # ${CRANE_INSECURE:+--insecure} mirrors GRYPE_REGISTRY_INSECURE_USE_HTTP + # for the job-local registry:2 service. + if command -v crane >/dev/null 2>&1; then + local d + d="$(crane digest ${CRANE_INSECURE:+--insecure} \ + "${repo}:${tag:-latest}" 2>/dev/null || true)" + [[ -n "$d" ]] && printf '%s@%s\n' "$repo" "$d" + fi + ;; + esac + # Best-effort: empty stdout means "couldn't resolve a digest" (the caller + # then scans without VEX). Never return non-zero — under `set -e` a + # non-zero return here would kill the script before that graceful path. + return 0 +} + +# --- Fetch VEX statements (tolerate absence) -------------------------------- +vex_args=() +vex_url="${VEX_BASE_URL}/pkg/oci/${VEX_REPO}/vex.json" +if curl -fsSL --retry 3 --max-time 30 "$vex_url" -o "$workdir/vex.json" 2>/dev/null \ + && jq -e .statements "$workdir/vex.json" >/dev/null 2>&1; then + stamp_ref="$(resolve_stamp_ref)" + if [[ -n "$stamp_ref" ]]; then + # Re-stamp every statement's product @id to the scanned digest. + jq --arg ref "$stamp_ref" \ + '.statements[].products[]."@id" = $ref' \ + "$workdir/vex.json" > "$workdir/vex.stamped.json" + echo "Applying VEX from $vex_url (re-stamped to $stamp_ref)" + vex_args=(--vex "$workdir/vex.stamped.json") + else + echo "::notice::VEX found at ${vex_url} but ${IMAGE_REF} is not digest-pinned and no repo digest could be resolved; scanning without VEX. Scan a registry:<repo>@<digest> ref (or install crane) to enable suppression." + fi +else + echo "::notice::No VEX statements published for ${VEX_REPO} (${vex_url}); scanning without VEX." +fi + +# --- Scan -------------------------------------------------------------------- +fail_args=() +if [[ -n "$FAIL_ON" ]]; then + fail_args=(--fail-on "$FAIL_ON") +fi + +echo "Scanning ${IMAGE_REF} ..." +rc=0 +"$GRYPE_CMD" "$IMAGE_REF" \ + ${vex_args[0]+"${vex_args[@]}"} \ + ${fail_args[0]+"${fail_args[@]}"} \ + -o "table=$workdir/table.txt" \ + -o "json=$workdir/report.json" \ + || rc=$? + +cat "$workdir/table.txt" 2>/dev/null || true + +# Grype failed to produce a usable report (e.g. the image could not be fetched +# or scanned). This is a scan error, not a CVE finding — only the *assessment* +# (findings vs FAIL_ON) is report-only, so a scan that can't run always fails, +# loudly, regardless of FAIL_ON. (VEX-resolution failures are handled earlier by +# scanning without VEX; this is the grype-can't-scan case.) +if ! jq -e '.matches' "$workdir/report.json" >/dev/null 2>&1; then + echo "::error::CVE scan of ${IMAGE_REF} failed: grype did not produce a report (exit ${rc})" >&2 + exit "$((rc == 0 ? 1 : rc))" +fi + +# --- Report ------------------------------------------------------------------ +critical=$(jq '[.matches[] | select(.vulnerability.severity == "Critical")] | length' "$workdir/report.json") +high=$(jq '[.matches[] | select(.vulnerability.severity == "High")] | length' "$workdir/report.json") +total=$(jq '.matches | length' "$workdir/report.json") + +# Per-location breakdown of Critical/High findings. Grype records the file a +# vulnerable package was found in (a bundled binary, or "" for OS packages), +# which is the key signal for "vulnerable code not in execute path" triage: +# a CVE that lives only in a build/scan tool is a strong not_affected case. +by_location=$(jq -r ' + [.matches[] + | select(.vulnerability.severity == "Critical" or .vulnerability.severity == "High") + | {loc: (.artifact.locations[0].path // "(os/image metadata)"), sev: .vulnerability.severity}] + | group_by(.loc)[] + | "| `\(.[0].loc)` | \(map(select(.sev == "Critical")) | length) | \(map(select(.sev == "High")) | length) |" +' "$workdir/report.json") + +if [[ -n "${GITHUB_STEP_SUMMARY:-}" ]]; then + { + echo "## CVE scan: \`${IMAGE_REF}\`" + echo + echo "| Critical | High | Total |" + echo "|---|---|---|" + echo "| ${critical} | ${high} | ${total} |" + if [[ -n "$by_location" ]]; then + echo + echo "### Critical/High by location" + echo + echo "| Location | Critical | High |" + echo "|---|---|---|" + echo "$by_location" + fi + echo + echo '```' + head -n 300 "$workdir/table.txt" + echo '```' + } >>"$GITHUB_STEP_SUMMARY" +fi + +if [[ "$critical" -gt 0 || "$high" -gt 0 ]]; then + echo "::warning::CVE scan of ${IMAGE_REF}: ${critical} critical, ${high} high (see job summary)" +fi + +if [[ "$rc" -ne 0 ]]; then + echo "::error::CVE scan of ${IMAGE_REF} failed the '${FAIL_ON}' severity gate" >&2 +fi +exit "$rc" diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kargo-cli-1.10.7/pkg/server/create_role_v1alpha1_test.go new/kargo-cli-1.10.8/pkg/server/create_role_v1alpha1_test.go --- old/kargo-cli-1.10.7/pkg/server/create_role_v1alpha1_test.go 2026-06-12 02:30:12.000000000 +0200 +++ new/kargo-cli-1.10.8/pkg/server/create_role_v1alpha1_test.go 2026-06-26 01:39:10.000000000 +0200 @@ -43,6 +43,18 @@ Namespace: testKargoRole.Namespace, }, } + // testGroupLessRole mirrors how the UI submits rules: apiGroups is omitted, so + // the server must resolve the group from the resource type. + testGroupLessRole := rbacapi.Role{ + ObjectMeta: metav1.ObjectMeta{ + Name: "group-less-role", + Namespace: testProject.Name, + }, + Rules: []rbacv1.PolicyRule{{ + Resources: []string{"stages"}, + Verbs: []string{"get"}, + }}, + } testRESTEndpoint( t, nil, http.MethodPost, "/v1beta1/projects/"+testProject.Name+"/roles", @@ -119,6 +131,26 @@ require.NoError(t, err) }, }, + { + name: "resolves omitted apiGroups from the resource type", + body: mustJSONBody(testGroupLessRole), + clientBuilder: fake.NewClientBuilder().WithObjects(testProject), + assertions: func(t *testing.T, w *httptest.ResponseRecorder, c client.Client) { + require.Equal(t, http.StatusCreated, w.Code) + + // The UI omits apiGroups; the server must resolve "stages" to the + // Kargo API group rather than leaving it in the core ("") group. + role := &rbacv1.Role{} + require.NoError(t, c.Get( + t.Context(), + client.ObjectKey{Namespace: testProject.Name, Name: testGroupLessRole.Name}, + role, + )) + require.Len(t, role.Rules, 1) + require.Equal(t, []string{kargoapi.GroupVersion.Group}, role.Rules[0].APIGroups) + require.Equal(t, []string{"stages"}, role.Rules[0].Resources) + }, + }, }, ) } diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kargo-cli-1.10.7/pkg/server/kubernetes/client.go new/kargo-cli-1.10.8/pkg/server/kubernetes/client.go --- old/kargo-cli-1.10.7/pkg/server/kubernetes/client.go 2026-06-12 02:30:12.000000000 +0200 +++ new/kargo-cli-1.10.8/pkg/server/kubernetes/client.go 2026-06-26 01:39:10.000000000 +0200 @@ -104,11 +104,17 @@ type AuthorizingClient struct{} -// The Client interface combines the familiar controller-runtime WithWatch -// interface with a helpful Authorized() method. -type Client interface { - libClient.WithWatch - +// Authorizer authorizes a single (verb, resource) operation for the user bound +// to the context. The API server's authorizing Kubernetes client satisfies this +// interface; it returns a forbidden error when the user is not permitted. +// +// This mirrors the subset of kubernetes.Client used to prevent privilege +// escalation when editing Kargo Roles: because the API server performs Role +// writes with its own (broadly privileged) ServiceAccount, Kubernetes' own +// escalation check runs against the server, not the user. We therefore verify -- +// as the user -- that the caller already holds every permission they are trying +// to grant. +type Authorizer interface { // Authorize attempts to authorize the user to perform the desired operation // on the specified resource. If the user is not authorized, an error is // returned. @@ -119,6 +125,14 @@ subresource string, key libClient.ObjectKey, ) error +} + +// The Client interface combines the familiar controller-runtime WithWatch +// interface with a helpful Authorized() method. +type Client interface { + libClient.WithWatch + + Authorizer // InternalClient returns the internal controller-runtime client used by this // client. This is useful for cases where the API server needs to bypass diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kargo-cli-1.10.7/pkg/server/rbac/escalation.go new/kargo-cli-1.10.8/pkg/server/rbac/escalation.go --- old/kargo-cli-1.10.7/pkg/server/rbac/escalation.go 1970-01-01 01:00:00.000000000 +0100 +++ new/kargo-cli-1.10.8/pkg/server/rbac/escalation.go 2026-06-26 01:39:10.000000000 +0200 @@ -0,0 +1,131 @@ +package rbac + +import ( + "context" + "fmt" + "strings" + + rbacv1 "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/akuity/kargo/pkg/server/kubernetes" +) + +// splitResourceType splits a resource type such as "freights/status" into its +// resource ("freights") and subresource ("status"). Resource types without a +// "/" have an empty subresource. +func splitResourceType(resourceType string) (resource, subresource string) { + resource, subresource, _ = strings.Cut(resourceType, "/") + return resource, subresource +} + +// verifyRulesNotEscalating returns an error unless the user bound to the context +// already holds every permission described by the provided rules in the given +// namespace. It is used when creating or replacing a Role's rules, when granting +// a permission to a Role, and (via verifyBindingNotEscalating) when binding a +// user to a Role. A nil authorizer (tests, or non-authorizing local mode) +// disables the check. As a convenience, it also returns the normalized rules to +// avoid the caller having to normalize them again for storage in the Role. +// +// The rules are normalized first, which reduces them to atomic permissions -- one +// API group, one resource, at most one resource name, with "*" verbs expanded -- +// so the authorization check is a flat loop over concrete permissions. +func verifyRulesNotEscalating( + ctx context.Context, + authz kubernetes.Authorizer, + namespace string, + rules []rbacv1.PolicyRule, +) ([]rbacv1.PolicyRule, error) { + normalized, err := NormalizePolicyRules( + rules, + &PolicyRuleNormalizationOptions{IncludeCustomVerbsInExpansion: true}, + ) + if err != nil { + return nil, err + } + if authz == nil { + return normalized, nil + } + + for _, rule := range normalized { + // Normalized rules carry exactly one API group and one resource. + resourceType := rule.Resources[0] + resource, subresource := splitResourceType(resourceType) + gvr := schema.GroupVersionResource{Group: rule.APIGroups[0], Resource: resource} + var name string + if len(rule.ResourceNames) > 0 { + name = rule.ResourceNames[0] + } + key := client.ObjectKey{Namespace: namespace, Name: name} + for _, verb := range rule.Verbs { + if err := authorizeGrant( + ctx, authz, verb, gvr, subresource, key, resourceType, + ); err != nil { + return nil, err + } + } + } + return normalized, nil +} + +// verifyBindingNotEscalating verifies that the requester already holds every +// permission conferred by the Roles bound to the given Kargo Role's +// ServiceAccount. It is used when binding a user's identity to a Kargo Role via +// claim annotations (GrantRoleToUsers): doing so maps that identity onto the +// ServiceAccount and grants it all of the Role's permissions, so a requester +// must not be able to confer more than they themselves hold. A nil authorizer +// disables the check. +// +// Only the namespaced Roles are considered: callers reach this after +// manageableResources, which rejects ServiceAccounts bound to ClusterRoles, so +// resources.ClusterRoles is empty here. +func verifyBindingNotEscalating( + ctx context.Context, + authz kubernetes.Authorizer, + namespace string, + resources Resources, +) error { + if authz == nil { + return nil + } + var rules []rbacv1.PolicyRule + for _, role := range resources.Roles { + rules = append(rules, role.Rules...) + } + _, err := verifyRulesNotEscalating(ctx, authz, namespace, rules) + return err +} + +// authorizeGrant checks a single granted permission against the user's own +// authority. A forbidden result is rewritten as an escalation error; any other +// error is propagated so the operation fails closed. +func authorizeGrant( + ctx context.Context, + authz kubernetes.Authorizer, + verb string, + gvr schema.GroupVersionResource, + subresource string, + key client.ObjectKey, + resourceType string, +) error { + err := authz.Authorize(ctx, verb, gvr, subresource, key) + if err == nil { + return nil + } + if apierrors.IsForbidden(err) { + return apierrors.NewForbidden( + gvr.GroupResource(), + key.Name, + fmt.Errorf( + "requester may not grant permissions it does not hold: verb %q on resource %q", + verb, resourceType, + ), + ) + } + return fmt.Errorf( + "error verifying permission to grant verb %q on resource %q: %w", + verb, resourceType, err, + ) +} diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kargo-cli-1.10.7/pkg/server/rbac/escalation_test.go new/kargo-cli-1.10.8/pkg/server/rbac/escalation_test.go --- old/kargo-cli-1.10.7/pkg/server/rbac/escalation_test.go 1970-01-01 01:00:00.000000000 +0100 +++ new/kargo-cli-1.10.8/pkg/server/rbac/escalation_test.go 2026-06-26 01:39:10.000000000 +0200 @@ -0,0 +1,269 @@ +package rbac + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + rbacapi "github.com/akuity/kargo/api/rbac/v1alpha1" + kargoapi "github.com/akuity/kargo/api/v1alpha1" +) + +// fakeAuthorizer is a test Authorizer. When allow is nil, everything is +// permitted; otherwise allow decides. Every call is recorded. +type fakeAuthorizer struct { + allow func(verb string, gvr schema.GroupVersionResource, subresource string) bool + calls []string +} + +func (f *fakeAuthorizer) Authorize( + _ context.Context, + verb string, + gvr schema.GroupVersionResource, + subresource string, + key client.ObjectKey, +) error { + res := gvr.Resource + if subresource != "" { + res += "/" + subresource + } + f.calls = append(f.calls, verb+" "+gvr.Group+"/"+res) + if f.allow == nil || f.allow(verb, gvr, subresource) { + return nil + } + return apierrors.NewForbidden(gvr.GroupResource(), key.Name, errors.New("not allowed")) +} + +// authorizingFakeClient is both a controller-runtime client and an Authorizer, +// so NewKubernetesRolesDatabase wires the escalation check to it. +type authorizingFakeClient struct { + client.Client + authz *fakeAuthorizer +} + +func (a *authorizingFakeClient) Authorize( + ctx context.Context, + verb string, + gvr schema.GroupVersionResource, + subresource string, + key client.ObjectKey, +) error { + return a.authz.Authorize(ctx, verb, gvr, subresource, key) +} + +func TestSplitResourceType(t *testing.T) { + r, s := splitResourceType("secrets") + require.Equal(t, "secrets", r) + require.Empty(t, s) + + r, s = splitResourceType("freights/status") + require.Equal(t, "freights", r) + require.Equal(t, "status", s) +} + +func TestVerifyRulesNotEscalating(t *testing.T) { + rules := []rbacv1.PolicyRule{ + { + APIGroups: []string{kargoapi.GroupVersion.Group}, + Resources: []string{"stages"}, + Verbs: []string{"get", "promote"}, + }, + { + APIGroups: []string{""}, + Resources: []string{"secrets"}, + Verbs: []string{"get"}, + }, + } + + t.Run("permitted when user holds every rule", func(t *testing.T) { + authz := &fakeAuthorizer{} + _, err := verifyRulesNotEscalating(t.Context(), authz, testProject, rules) + require.NoError(t, err) + require.ElementsMatch( + t, + []string{ + "get " + kargoapi.GroupVersion.Group + "/stages", + "promote " + kargoapi.GroupVersion.Group + "/stages", + "get /secrets", + }, + authz.calls, + ) + }) + + t.Run("forbidden when user lacks one rule", func(t *testing.T) { + authz := &fakeAuthorizer{ + allow: func(_ string, gvr schema.GroupVersionResource, _ string) bool { + return gvr.Resource != "secrets" // user has no secrets access + }, + } + _, err := verifyRulesNotEscalating(t.Context(), authz, testProject, rules) + require.Error(t, err) + require.True(t, apierrors.IsForbidden(err)) + require.ErrorContains(t, err, `resource "secrets"`) + }) +} + +func TestNewKubernetesRolesDatabaseWiresAuthorizer(t *testing.T) { + plain := fake.NewClientBuilder().WithScheme(scheme).Build() + + db, ok := NewKubernetesRolesDatabase(plain, plain, RolesDatabaseConfig{}).(*rolesDatabase) + require.True(t, ok) + require.Nil(t, db.authorizer, "a plain client must leave the authorizer nil") + + authorizing := &authorizingFakeClient{Client: plain, authz: &fakeAuthorizer{}} + db, ok = NewKubernetesRolesDatabase(authorizing, plain, RolesDatabaseConfig{}).(*rolesDatabase) + require.True(t, ok) + require.NotNil(t, db.authorizer, "a client implementing Authorizer must populate the authorizer") +} + +func TestVerifyBindingNotEscalating(t *testing.T) { + resources := Resources{ + Roles: []rbacv1.Role{{ + Rules: []rbacv1.PolicyRule{ + {APIGroups: []string{""}, Resources: []string{"configmaps", "secrets"}, Verbs: []string{"*"}}, + }, + }}, + } + + t.Run("nil authorizer disables the check", func(t *testing.T) { + require.NoError(t, verifyBindingNotEscalating(t.Context(), nil, testProject, resources)) + }) + + t.Run("no bound roles means nothing to escalate", func(t *testing.T) { + require.NoError(t, verifyBindingNotEscalating(t.Context(), &fakeAuthorizer{}, testProject, Resources{})) + }) + + t.Run("permitted when requester holds the role's permissions", func(t *testing.T) { + require.NoError(t, verifyBindingNotEscalating(t.Context(), &fakeAuthorizer{}, testProject, resources)) + }) + + t.Run("forbidden when requester lacks a permission in the bound role", func(t *testing.T) { + authz := &fakeAuthorizer{ + allow: func(_ string, gvr schema.GroupVersionResource, _ string) bool { + return gvr.Resource != "secrets" + }, + } + err := verifyBindingNotEscalating(t.Context(), authz, testProject, resources) + require.Error(t, err) + require.True(t, apierrors.IsForbidden(err)) + require.ErrorContains(t, err, `resource "secrets"`) + }) +} + +// TestGrantRoleToUsersBlocksEscalation is the end-to-end guard: a user who +// cannot read secrets must not be able to bind an identity onto a Role that can. +func TestGrantRoleToUsersBlocksEscalation(t *testing.T) { + secretsRole := []rbacv1.PolicyRule{{ + APIGroups: []string{""}, + Resources: []string{"secrets"}, + Verbs: []string{"*"}, + }} + claims := []rbacapi.Claim{{Name: "email", Values: []string{"[email protected]"}}} + + newDB := func(authz *fakeAuthorizer) (RolesDatabase, client.Client) { + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects( + managedServiceAccount(nil), + managedRole(secretsRole), + managedRoleBinding(), + ).Build() + return NewKubernetesRolesDatabase( + &authorizingFakeClient{Client: c, authz: authz}, c, RolesDatabaseConfig{}, + ), c + } + + t.Run("blocked when requester lacks the role's permissions", func(t *testing.T) { + db, c := newDB(&fakeAuthorizer{ + allow: func(_ string, gvr schema.GroupVersionResource, _ string) bool { + return gvr.Resource != "secrets" + }, + }) + _, err := db.GrantRoleToUsers(t.Context(), testProject, testKargoRoleName, claims) + require.Error(t, err) + require.True(t, apierrors.IsForbidden(err)) + + // The claim must not have been written to the ServiceAccount. + sa := &corev1.ServiceAccount{} + require.NoError(t, c.Get(t.Context(), objKey, sa)) + got, err := rbacapi.OIDCClaimsFromAnnotationValues(sa.Annotations) + require.NoError(t, err) + require.Empty(t, got) + }) + + t.Run("allowed when requester holds the role's permissions", func(t *testing.T) { + db, c := newDB(&fakeAuthorizer{}) // permits everything + _, err := db.GrantRoleToUsers(t.Context(), testProject, testKargoRoleName, claims) + require.NoError(t, err) + + sa := &corev1.ServiceAccount{} + require.NoError(t, c.Get(t.Context(), objKey, sa)) + got, err := rbacapi.OIDCClaimsFromAnnotationValues(sa.Annotations) + require.NoError(t, err) + require.Equal(t, []string{"[email protected]"}, got["email"]) + }) +} + +// TestGrantPermissionsToRoleBlocksEscalation is the end-to-end guard: a user who +// cannot read secrets must not be able to grant secrets access via a Role edit. +func TestGrantPermissionsToRoleBlocksEscalation(t *testing.T) { + newDB := func(authz *fakeAuthorizer) (RolesDatabase, client.Client) { + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects( + managedServiceAccount(nil), + ).Build() + db := NewKubernetesRolesDatabase( + &authorizingFakeClient{Client: c, authz: authz}, c, RolesDatabaseConfig{}, + ) + // A fake client has no discovery, so stub group resolution. + rdb, ok := db.(*rolesDatabase) + require.True(t, ok) + rdb.resolveGroup = fakeGroupResolver + return db, c + } + + t.Run("blocked when requester lacks secrets", func(t *testing.T) { + db, c := newDB(&fakeAuthorizer{ + allow: func(_ string, gvr schema.GroupVersionResource, _ string) bool { + return gvr.Resource != "secrets" + }, + }) + _, err := db.GrantPermissionsToRole( + t.Context(), testProject, testKargoRoleName, + &rbacapi.ResourceDetails{ResourceType: "secrets", Verbs: []string{"get", "list"}}, + ) + require.Error(t, err) + require.True(t, apierrors.IsForbidden(err)) + require.ErrorContains(t, err, "may not grant permissions it does not hold") + + // The Role must not have been created as a side effect. + err = c.Get(t.Context(), objKey, &rbacv1.Role{}) + require.True(t, apierrors.IsNotFound(err)) + }) + + t.Run("allowed when requester holds the permission", func(t *testing.T) { + db, c := newDB(&fakeAuthorizer{}) // permits everything + _, err := db.GrantPermissionsToRole( + t.Context(), testProject, testKargoRoleName, + &rbacapi.ResourceDetails{ResourceType: "secrets", Verbs: []string{"get", "list"}}, + ) + require.NoError(t, err) + + role := &rbacv1.Role{} + require.NoError(t, c.Get(t.Context(), objKey, role)) + require.Equal( + t, + []rbacv1.PolicyRule{{ + APIGroups: []string{""}, + Resources: []string{"secrets"}, + Verbs: []string{"get", "list"}, + }}, + role.Rules, + ) + }) +} diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kargo-cli-1.10.7/pkg/server/rbac/policy_rules.go new/kargo-cli-1.10.8/pkg/server/rbac/policy_rules.go --- old/kargo-cli-1.10.7/pkg/server/rbac/policy_rules.go 2026-06-12 02:30:12.000000000 +0200 +++ new/kargo-cli-1.10.8/pkg/server/rbac/policy_rules.go 2026-06-26 01:39:10.000000000 +0200 @@ -7,9 +7,8 @@ rbacv1 "k8s.io/api/rbac/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" - - rolloutsapi "github.com/akuity/kargo/api/stubs/rollouts/v1alpha1" - kargoapi "github.com/akuity/kargo/api/v1alpha1" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime/schema" ) var ( @@ -77,23 +76,32 @@ ) (map[string]rbacv1.PolicyRule, error) { rulesMap := make(map[string]rbacv1.PolicyRule) for _, rule := range rules { + // Preserve the group(s) the rule specifies rather than overwriting them; + // an absent group means the core ("") group. (Previously the group was + // replaced using a hardcoded table, which could not account for resource + // types from Kargo's enterprise APIs or other CRDs. Callers that start from + // a group-less request resolve the group up front -- see groupResolver.) + groups := rule.APIGroups + if len(groups) == 0 { + groups = []string{""} + } + names := rule.ResourceNames + if len(names) == 0 { + names = []string{""} + } for _, resource := range rule.Resources { if err := validateResourceTypeName(resource); err != nil { return nil, err } - // We ignore the group in the rule and use what we know to be the correct - // group for the resource type. - group := getGroupName(resource) - if len(rule.ResourceNames) == 0 { - rule.ResourceNames = append(rule.ResourceNames, "") - } - for _, resourceName := range rule.ResourceNames { - verbs := rule.Verbs - key := RuleKey(group, resource, resourceName) - if existingRule, ok := rulesMap[key]; ok { - verbs = append(existingRule.Verbs, verbs...) + for _, group := range groups { + for _, resourceName := range names { + verbs := rule.Verbs + key := RuleKey(group, resource, resourceName) + if existingRule, ok := rulesMap[key]; ok { + verbs = append(existingRule.Verbs, verbs...) + } + rulesMap[key] = buildRule(group, resource, resourceName, verbs, opts) } - rulesMap[key] = buildRule(group, resource, resourceName, verbs, opts) } } } @@ -177,43 +185,113 @@ return rule } -// nolint: goconst +// validateResourceTypeName performs a best-effort check that resource is the plural form of a +// resource type. It deliberately does not validate against a fixed set of known types because +// otherwise we have to maintain a hardcoded table of all known resource types. Kubernetes resource +// names are conventionally the lowercase plural of a Kind and therefore end in "s"; an input that +// does not is almost certainly a singular fat-finger (e.g. "stage" instead of "stages"), so we +// reject it with a suggestion rather than silently creating a rule that matches nothing. func validateResourceTypeName(resource string) error { - switch resource { - case "analysisruns", "analysistemplates", "configmaps", "events", "freights", - "freights/status", "projectconfigs", "promotions", "rolebindings", "roles", - "secrets", "serviceaccounts", "stages", "warehouses", "promotiontasks": + // Subresources (e.g. "freights/status") are validated on their resource part. + base, subresource, hasSubresource := strings.Cut(resource, "/") + if base == "" { + return apierrors.NewBadRequest(fmt.Sprintf("unrecognized resource type %q", resource)) + } + if strings.HasSuffix(base, "s") { return nil - case "analysisrun", "analysistemplate", "configmap", "event", "freight", - "projectconfig", "promotion", "role", "rolebinding", "secret", - "serviceaccount", "stage", "warehouse": - return apierrors.NewBadRequest( - fmt.Sprintf(`unrecognized resource type %q; did you mean "%ss"?`, resource, resource), - ) - case "freight/status": - return apierrors.NewBadRequest( - `unrecognized resource type "freight/status"; did you mean "freights/status"?`, - ) - default: - return apierrors.NewBadRequest(fmt.Sprintf(`unrecognized resource type %q`, resource)) } + // Looks singular; suggest the plural form using Kubernetes' own guess. + plural, _ := meta.UnsafeGuessKindToResource(schema.GroupVersionKind{Kind: base}) + suggestion := plural.Resource + if hasSubresource { + suggestion += "/" + subresource + } + return apierrors.NewBadRequest(fmt.Sprintf( + "unrecognized resource type %q; did you mean %q?", resource, suggestion, + )) } -// nolint: goconst -func getGroupName(resourceType string) string { - // resourceType must already be validated - switch resourceType { - case "events", "secrets", "serviceaccounts": - return "" - case "rolebindings", "roles": - return rbacv1.SchemeGroupVersion.Group - case "freights", "freights/status", "promotions", "stages", "warehouses": - return kargoapi.GroupVersion.Group - case "analysisruns", "analysistemplates": - return rolloutsapi.GroupVersion.Group - default: - return "" // If the resourceType was validated, this will never happen +// groupResolver resolves the API group that serves a (plural) resource type. It +// is consulted only when a request carries no explicit group -- the Grant/Revoke +// ResourceDetails flow, and group-less rules submitted via Create/Update (see +// resolveRuleGroups). +type groupResolver func(resource string) (group string, err error) + +// newRESTMapperGroupResolver returns a groupResolver backed by a RESTMapper. The +// API server's RESTMapper is discovery-backed, so this resolves the group for +// any resource type the cluster actually serves -- core, CRDs, and Kargo's +// enterprise APIs alike -- without a hardcoded table or configuration. Known +// resources are served from the mapper's in-memory cache; it returns a +// bad-request error for an unknown or ambiguous resource type. +func newRESTMapperGroupResolver(mapper meta.RESTMapper) groupResolver { + return func(resource string) (string, error) { + // The group is a property of the resource, not the subresource. + res, _, _ := strings.Cut(resource, "/") + gvrs, err := mapper.ResourcesFor(schema.GroupVersionResource{Resource: res}) + if err != nil { + if meta.IsNoMatchError(err) { + return "", apierrors.NewBadRequest(fmt.Sprintf("unrecognized resource type %q", resource)) + } + return "", fmt.Errorf("error resolving API group for resource type %q: %w", resource, err) + } + groups := make(map[string]struct{}, len(gvrs)) + for _, gvr := range gvrs { + groups[gvr.Group] = struct{}{} + } + switch len(groups) { + case 1: + var group string + // NOTE(thomastaylor312): I know this looks weird, but it's the only way to get the + // single key out of a map in Go. + for g := range groups { + group = g + } + return group, nil + case 0: + return "", apierrors.NewBadRequest(fmt.Sprintf("unrecognized resource type %q", resource)) + default: + return "", apierrors.NewBadRequest(fmt.Sprintf( + "ambiguous resource type %q; it is served by multiple API groups", resource, + )) + } + } +} + +// resolveRuleGroups fills in the API group of any rule that does not specify +// one, by resolving it from the rule's resource type(s). Rules that already +// specify a group are returned unchanged. A group-less rule that lists multiple +// resources is split so each resource is paired with its own resolved group, +// since different resources may live in different groups. A nil resolver leaves +// rules unchanged. +// +// This is the entry point for group-less rules submitted through Create/Update +// (notably the UI, which omits apiGroups and relies on the server to resolve +// them); NormalizePolicyRules itself only preserves groups, never resolves them. +func resolveRuleGroups( + rules []rbacv1.PolicyRule, + resolve groupResolver, +) ([]rbacv1.PolicyRule, error) { + if resolve == nil { + return rules, nil + } + resolved := make([]rbacv1.PolicyRule, 0, len(rules)) + for _, rule := range rules { + if len(rule.APIGroups) > 0 || len(rule.Resources) == 0 { + resolved = append(resolved, rule) + continue + } + for _, resource := range rule.Resources { + group, err := resolve(resource) + if err != nil { + return nil, err + } + r := *rule.DeepCopy() + r.APIGroups = []string{group} + r.Resources = []string{resource} + resolved = append(resolved, r) + } } + return resolved, nil } func allVerbsFor(resourceType string, includeCustom bool) []string { diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kargo-cli-1.10.7/pkg/server/rbac/policy_rules_test.go new/kargo-cli-1.10.8/pkg/server/rbac/policy_rules_test.go --- old/kargo-cli-1.10.7/pkg/server/rbac/policy_rules_test.go 2026-06-12 02:30:12.000000000 +0200 +++ new/kargo-cli-1.10.8/pkg/server/rbac/policy_rules_test.go 2026-06-26 01:39:10.000000000 +0200 @@ -1,6 +1,7 @@ package rbac import ( + "errors" "testing" "github.com/stretchr/testify/require" @@ -163,10 +164,10 @@ ) }) - t.Run("correct groups are determined automatically", func(t *testing.T) { + t.Run("groups are preserved", func(t *testing.T) { rules, err := NormalizePolicyRules( []rbacv1.PolicyRule{{ - APIGroups: []string{"", "foo", "bar"}, + APIGroups: []string{kargoapi.GroupVersion.Group}, Resources: []string{"stages"}, Verbs: []string{"get"}, }}, @@ -175,10 +176,36 @@ require.NoError(t, err) require.Equal( t, + []rbacv1.PolicyRule{{ + APIGroups: []string{kargoapi.GroupVersion.Group}, + Resources: []string{"stages"}, + Verbs: []string{"get"}, + }}, + rules, + ) + }) + + t.Run("multiple groups expand", func(t *testing.T) { + rules, err := NormalizePolicyRules( + []rbacv1.PolicyRule{{ + APIGroups: []string{"foo", "bar"}, + Resources: []string{"widgets"}, + Verbs: []string{"get"}, + }}, + nil, + ) + require.NoError(t, err) + require.Equal( + t, []rbacv1.PolicyRule{ { - APIGroups: []string{kargoapi.GroupVersion.Group}, - Resources: []string{"stages"}, + APIGroups: []string{"bar"}, + Resources: []string{"widgets"}, + Verbs: []string{"get"}, + }, + { + APIGroups: []string{"foo"}, + Resources: []string{"widgets"}, Verbs: []string{"get"}, }, }, @@ -189,16 +216,21 @@ t.Run("kitchen sink", func(t *testing.T) { rules, err := NormalizePolicyRules( []rbacv1.PolicyRule{ - { // Never mind that this doesn't make sense. It should all get fixed + { // group "" is preserved as core (not inferred); splits per resource APIGroups: []string{""}, Resources: []string{"serviceaccounts", "stages"}, Verbs: []string{"*"}, }, - { // These should get de-duped + { // merges with the get-only kargo stages rule below APIGroups: []string{kargoapi.GroupVersion.Group}, Resources: []string{"stages"}, Verbs: []string{"*"}, }, + { // merged with the stages rule above (same group + resource) + APIGroups: []string{kargoapi.GroupVersion.Group}, + Resources: []string{"stages"}, + Verbs: []string{"get"}, + }, { APIGroups: []string{kargoapi.GroupVersion.Group}, Resources: []string{"warehouses"}, @@ -218,6 +250,16 @@ Verbs: allVerbs, }, { + // The group "" stages rule is preserved as core -- it is NOT + // inferred to kargo.akuity.io and so does NOT merge with the + // kargo.akuity.io stages rule below. (Group inference for + // group-less input happens at the Create/Update layer via + // resolveRuleGroups, not in NormalizePolicyRules.) + APIGroups: []string{""}, + Resources: []string{"stages"}, + Verbs: allStagesVerbs, + }, + { APIGroups: []string{kargoapi.GroupVersion.Group}, Resources: []string{"stages"}, Verbs: allStagesVerbs, @@ -238,5 +280,80 @@ rules, ) }) +} + +func TestResolveRuleGroups(t *testing.T) { + resolve := func(resource string) (string, error) { + switch resource { + case "stages": + return kargoapi.GroupVersion.Group, nil + case "secrets": + return "", nil + default: + return "", errors.New("unrecognized resource type") + } + } + + t.Run("nil resolver leaves rules unchanged", func(t *testing.T) { + in := []rbacv1.PolicyRule{{Resources: []string{"stages"}, Verbs: []string{"get"}}} + out, err := resolveRuleGroups(in, nil) + require.NoError(t, err) + require.Equal(t, in, out) + }) + + t.Run("explicit group is preserved", func(t *testing.T) { + in := []rbacv1.PolicyRule{{ + APIGroups: []string{"already.set"}, + Resources: []string{"stages"}, + Verbs: []string{"get"}, + }} + out, err := resolveRuleGroups(in, resolve) + require.NoError(t, err) + require.Equal(t, in, out) + }) + + t.Run("empty group is resolved from the resource", func(t *testing.T) { + out, err := resolveRuleGroups([]rbacv1.PolicyRule{{ + Resources: []string{"stages"}, + Verbs: []string{"get"}, + }}, resolve) + require.NoError(t, err) + require.Equal(t, []rbacv1.PolicyRule{{ + APIGroups: []string{kargoapi.GroupVersion.Group}, + Resources: []string{"stages"}, + Verbs: []string{"get"}, + }}, out) + }) + + t.Run("group-less multi-resource rule splits per resource", func(t *testing.T) { + out, err := resolveRuleGroups([]rbacv1.PolicyRule{{ + Resources: []string{"stages", "secrets"}, + Verbs: []string{"get"}, + }}, resolve) + require.NoError(t, err) + require.Equal( + t, + []rbacv1.PolicyRule{ + { + APIGroups: []string{kargoapi.GroupVersion.Group}, + Resources: []string{"stages"}, + Verbs: []string{"get"}, + }, + { + APIGroups: []string{""}, + Resources: []string{"secrets"}, + Verbs: []string{"get"}, + }, + }, + out, + ) + }) + t.Run("resolver error is propagated", func(t *testing.T) { + _, err := resolveRuleGroups([]rbacv1.PolicyRule{{ + Resources: []string{"unknown"}, + Verbs: []string{"get"}, + }}, resolve) + require.Error(t, err) + }) } diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kargo-cli-1.10.7/pkg/server/rbac/roles.go new/kargo-cli-1.10.8/pkg/server/rbac/roles.go --- old/kargo-cli-1.10.7/pkg/server/rbac/roles.go 2026-06-12 02:30:12.000000000 +0200 +++ new/kargo-cli-1.10.8/pkg/server/rbac/roles.go 2026-06-26 01:39:10.000000000 +0200 @@ -17,6 +17,7 @@ rbacapi "github.com/akuity/kargo/api/rbac/v1alpha1" kargoapi "github.com/akuity/kargo/api/v1alpha1" + "github.com/akuity/kargo/pkg/server/kubernetes" ) type RolesDatabaseConfig struct { @@ -163,7 +164,15 @@ type rolesDatabase struct { client client.Client internalClient client.Client - cfg RolesDatabaseConfig + // NOTE(thomastaylor312): this just ends up being the same client as `client`, but we have it as + // a separate field so testing is easier because we can nil this out or mock what we need + authorizer kubernetes.Authorizer + // resolveGroup resolves the API group for a (plural) resource type when a + // request does not specify one (the Grant/Revoke ResourceDetails flow). It is + // derived from the client's discovery-backed RESTMapper in the constructor; + // tests may override it. + resolveGroup groupResolver + cfg RolesDatabaseConfig } // NewKubernetesRolesDatabase returns an implementation of the RolesDatabase @@ -175,11 +184,27 @@ internalClient client.Client, cfg RolesDatabaseConfig, ) RolesDatabase { - return &rolesDatabase{ + db := &rolesDatabase{ client: c, internalClient: internalClient, cfg: cfg, } + // The API server passes its authorizing client here. We use it to verify -- + // as the requesting user -- that a Role edit does not grant permissions the + // user does not already hold. This is necessary because Role writes are + // performed with the API server's own broadly-privileged ServiceAccount, so + // Kubernetes' built-in escalation prevention runs against the server rather + // than the user. Clients that do not implement Authorizer (test fakes, or the + // non-authorizing local-mode client whose writes are already constrained by + // the user's own credentials) leave this nil, disabling the check. + if authz, ok := c.(kubernetes.Authorizer); ok { + db.authorizer = authz + } + // Resolve API groups for group-less requests via the client's discovery-backed + // RESTMapper, so any served resource type (core, CRDs, enterprise APIs) is + // covered without a hardcoded table. + db.resolveGroup = newRESTMapperGroupResolver(c.RESTMapper()) + return db } // CreateRole implements the RolesDatabase interface. @@ -243,11 +268,29 @@ // If we get to here, we may proceed with creating the // ServiceAccount/Role/RoleBinding trio - sa, role, rb, err := RoleToResources(kargoRole) + // Fill in the API group of any rule that omits it (the UI sends group-less + // rules and relies on the server to resolve them from the resource type). + resolvedRules, err := resolveRuleGroups(kargoRole.Rules, c.resolveGroup) + if err != nil { + return nil, err + } + kargoRole.Rules = resolvedRules + + // NOTE(thomastaylor312): This normalizes the rules and then the verify step does it again. This + // is probably fine for now, but calling it out here in case there is a problem in the future + sa, role, rb, err = RoleToResources(kargoRole) if err != nil { return nil, fmt.Errorf("error converting Kargo Role to resources: %w", err) } + // Reject the create unless the requester already holds every permission in + // the new Role's rule set (prevents privilege escalation via Role creation). + if _, err = verifyRulesNotEscalating( + ctx, c.authorizer, kargoRole.Namespace, role.Rules, + ); err != nil { + return nil, err + } + // Append the description annotation to the Role if it exists if description, ok := kargoRole.Annotations[kargoapi.AnnotationKeyDescription]; ok { if sa.Annotations == nil { @@ -468,17 +511,9 @@ return nil, err } - group := getGroupName(resourceDetails.ResourceType) - - // Deal with wildcard verb - for _, verb := range resourceDetails.Verbs { - if strings.TrimSpace(verb) == "*" { - resourceDetails.Verbs = append( - resourceDetails.Verbs, - allVerbsFor(resourceDetails.ResourceType, true)..., - ) - break - } + group, err := c.resolveGroup(resourceDetails.ResourceType) + if err != nil { + return nil, err } newRole := role @@ -493,7 +528,22 @@ if resourceDetails.ResourceName != "" { newRule.ResourceNames = []string{resourceDetails.ResourceName} } - if newRole.Rules, err = NormalizePolicyRules(append(newRole.Rules, newRule), nil); err != nil { + + // Reject the grant unless the requester already holds every permission they + // are attempting to confer (prevents privilege escalation via Role editing). + // Only the new rule is checked; pre-existing rules were vetted when added. + // The normalized form is discarded here because storage needs the new rule + // merged with the role's existing rules, not the delta on its own. + if _, err = verifyRulesNotEscalating( + ctx, c.authorizer, project, []rbacv1.PolicyRule{newRule}, + ); err != nil { + return nil, err + } + + if newRole.Rules, err = NormalizePolicyRules( + append(newRole.Rules, newRule), + &PolicyRuleNormalizationOptions{IncludeCustomVerbsInExpansion: true}, + ); err != nil { return nil, fmt.Errorf("error normalizing RBAC policy rules: %w", err) } @@ -533,6 +583,13 @@ if _, _, err = manageableResources(resources); err != nil { return nil, err } + // Binding a user to a Kargo Role maps their identity onto its ServiceAccount, + // conferring the Role's permissions. Reject unless the requester already holds + // those permissions (prevents privilege escalation by binding a user -- + // possibly the requester themselves -- to a more powerful Role). + if err = verifyBindingNotEscalating(ctx, c.authorizer, project, resources); err != nil { + return nil, err + } if err = amendClaimAnnotations( resources.ServiceAccount, claimListToMap(claims), @@ -581,7 +638,6 @@ namespace, saList.Items[i].Name, ) - if err != nil { return nil, fmt.Errorf( "error getting underlying resources for Kargo Role %q from namespace %q: %w", @@ -683,7 +739,10 @@ return nil, err } - group := getGroupName(resourceDetails.ResourceType) + group, err := c.resolveGroup(resourceDetails.ResourceType) + if err != nil { + return nil, err + } filteredRules := make([]rbacv1.PolicyRule, 0, len(role.Rules)) for _, rule := range role.Rules { @@ -787,23 +846,34 @@ ) } + newRole := role + if newRole == nil { + newRole = buildNewRole(kargoRole.Namespace, kargoRole.Name) + } + // Fill in the API group of any rule that omits it (the UI sends group-less + // rules and relies on the server to resolve them from the resource type). + resolvedRules, err := resolveRuleGroups(kargoRole.Rules, c.resolveGroup) + if err != nil { + return nil, err + } + // Reject the update unless the requester already holds every permission in + // the resulting rule set (prevents privilege escalation via Role editing). + // The check runs before anything is persisted, so a denied update has no + // side effects. + normalized, err := verifyRulesNotEscalating( + ctx, c.authorizer, kargoRole.Namespace, resolvedRules, + ) + if err != nil { + return nil, err + } + newRole.Rules = normalized + if err = c.client.Update(ctx, resources.ServiceAccount); err != nil { return nil, fmt.Errorf( "error updating ServiceAccount %q in namespace %q: %w", kargoRole.Name, kargoRole.Namespace, err, ) } - - newRole := role - if newRole == nil { - newRole = buildNewRole(kargoRole.Namespace, kargoRole.Name) - } - if newRole.Rules, err = NormalizePolicyRules( - kargoRole.Rules, - &PolicyRuleNormalizationOptions{IncludeCustomVerbsInExpansion: true}, - ); err != nil { - return nil, fmt.Errorf("error normalizing RBAC policy rules: %w", err) - } if role == nil { if err := c.client.Create(ctx, newRole); err != nil { return nil, fmt.Errorf("error creating Role %q in namespace %q: %w", kargoRole.Name, kargoRole.Namespace, err) @@ -860,7 +930,8 @@ } for name, values := range claims { - kargoRole.Claims = append(kargoRole.Claims, + kargoRole.Claims = append( + kargoRole.Claims, rbacapi.Claim{ Name: name, Values: values, diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kargo-cli-1.10.7/pkg/server/rbac/roles_test.go new/kargo-cli-1.10.8/pkg/server/rbac/roles_test.go --- old/kargo-cli-1.10.7/pkg/server/rbac/roles_test.go 2026-06-12 02:30:12.000000000 +0200 +++ new/kargo-cli-1.10.8/pkg/server/rbac/roles_test.go 2026-06-26 01:39:10.000000000 +0200 @@ -2,7 +2,9 @@ import ( "context" + "fmt" "maps" + "strings" "testing" "time" @@ -30,6 +32,23 @@ objKey = client.ObjectKey{Namespace: testProject, Name: testKargoRoleName} ) +// fakeGroupResolver stands in for the discovery-backed group resolver (which a +// fake client lacks), mapping the resource types used in tests to their API +// groups. It mirrors the mapping the old getGroupName table provided. +func fakeGroupResolver(resource string) (string, error) { + base, _, _ := strings.Cut(resource, "/") + switch base { + case "secrets", "serviceaccounts", "configmaps", "events": + return "", nil + case "roles", "rolebindings": + return rbacv1.SchemeGroupVersion.Group, nil + case "stages", "warehouses", "promotions", "freights", "projectconfigs", "promotiontasks": + return kargoapi.GroupVersion.Group, nil + default: + return "", apierrors.NewBadRequest(fmt.Sprintf("unrecognized resource type %q", resource)) + } +} + func init() { scheme = runtime.NewScheme() err := corev1.AddToScheme(scheme) @@ -512,6 +531,9 @@ managedServiceAccount(nil), ).Build() db := NewKubernetesRolesDatabase(c, c, RolesDatabaseConfig{}) + rdb, ok := db.(*rolesDatabase) + require.True(t, ok) + rdb.resolveGroup = fakeGroupResolver kargoRole, err := db.GrantPermissionsToRole( t.Context(), testProject, @@ -571,6 +593,9 @@ managedRoleBinding(), ).Build() db := NewKubernetesRolesDatabase(c, c, RolesDatabaseConfig{}) + rdb, ok := db.(*rolesDatabase) + require.True(t, ok) + rdb.resolveGroup = fakeGroupResolver kargoRole, err := db.GrantPermissionsToRole( t.Context(), testProject, @@ -929,6 +954,9 @@ managedRoleBinding(), ).Build() db := NewKubernetesRolesDatabase(c, c, RolesDatabaseConfig{}) + rdb, ok := db.(*rolesDatabase) + require.True(t, ok) + rdb.resolveGroup = fakeGroupResolver kargoRole, err := db.RevokePermissionsFromRole( t.Context(), testProject, diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kargo-cli-1.10.7/pkg/server/rest_test.go new/kargo-cli-1.10.8/pkg/server/rest_test.go --- old/kargo-cli-1.10.7/pkg/server/rest_test.go 2026-06-12 02:30:12.000000000 +0200 +++ new/kargo-cli-1.10.8/pkg/server/rest_test.go 2026-06-26 01:39:10.000000000 +0200 @@ -16,6 +16,7 @@ coordinationv1 "k8s.io/api/coordination/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" @@ -51,6 +52,21 @@ assertions func(*testing.T, *httptest.ResponseRecorder, client.Client) } +// testRESTMapper builds a RESTMapper from a scheme so fake clients can resolve a +// resource type's API group the way the production discovery-backed mapper does. +// It mirrors a real cluster by omitting rbac.kargo.akuity.io types, which are +// virtual API objects rather than resources the cluster actually serves. +func testRESTMapper(s *runtime.Scheme) meta.RESTMapper { + m := meta.NewDefaultRESTMapper(nil) + for gvk := range s.AllKnownTypes() { + if strings.HasSuffix(gvk.Kind, "List") || gvk.Group == rbacapi.GroupVersion.Group { + continue + } + m.Add(gvk, meta.RESTScopeNamespace) + } + return m +} + func testRESTEndpoint( t *testing.T, serverCfg *config.ServerConfig, @@ -96,7 +112,10 @@ if testCase.clientBuilder == nil { testCase.clientBuilder = fake.NewClientBuilder() } - internalClient := testCase.clientBuilder.WithScheme(testScheme).Build() + internalClient := testCase.clientBuilder. + WithScheme(testScheme). + WithRESTMapper(testRESTMapper(testScheme)). + Build() s.client, err = kubernetes.NewClient( t.Context(), &rest.Config{}, @@ -208,7 +227,10 @@ if testCase.clientBuilder == nil { testCase.clientBuilder = fake.NewClientBuilder() } - internalClient := testCase.clientBuilder.WithScheme(testScheme).Build() + internalClient := testCase.clientBuilder. + WithScheme(testScheme). + WithRESTMapper(testRESTMapper(testScheme)). + Build() var err error s.client, err = kubernetes.NewClient( t.Context(), diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kargo-cli-1.10.7/ui/src/features/project/settings/views/project-config/project-config.tsx new/kargo-cli-1.10.8/ui/src/features/project/settings/views/project-config/project-config.tsx --- old/kargo-cli-1.10.7/ui/src/features/project/settings/views/project-config/project-config.tsx 2026-06-12 02:30:12.000000000 +0200 +++ new/kargo-cli-1.10.8/ui/src/features/project/settings/views/project-config/project-config.tsx 2026-06-26 01:39:10.000000000 +0200 @@ -14,7 +14,7 @@ import { useModal } from '@ui/features/common/modal/use-modal'; import { projectConfigYAMLExample } from '@ui/features/project/list/utils/project-yaml-example'; import { useGetProjectConfig } from '@ui/gen/api/v2/core/core'; -import { useUpdateResource } from '@ui/gen/api/v2/resources/resources'; +import { useCreateResource, useUpdateResource } from '@ui/gen/api/v2/resources/resources'; import projectConfigSchema from '@ui/gen/schema/projectconfigs.kargo.akuity.io_v1alpha1.json'; import { zodValidators } from '@ui/utils/validators'; @@ -73,14 +73,19 @@ resolver: zodResolver(formSchema) }); - const createOrUpdateMutation = useUpdateResource({ + const mutationOptions = { mutation: { onSuccess: () => { message.success({ content: `ProjectConfig has been ${creation ? 'created' : 'updated'}` }); projectConfigQuery.refetch(); } } - }); + }; + + const createMutation = useCreateResource(mutationOptions); + const updateMutation = useUpdateResource(mutationOptions); + + const createOrUpdateMutation = creation ? createMutation : updateMutation; const onSubmitConfig = projectConfigForm.handleSubmit((data) => createOrUpdateMutation.mutate({ data: data.value }) diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/kargo-cli-1.10.7/ui/src/features/settings/cluster-config/cluster-config.tsx new/kargo-cli-1.10.8/ui/src/features/settings/cluster-config/cluster-config.tsx --- old/kargo-cli-1.10.7/ui/src/features/settings/cluster-config/cluster-config.tsx 2026-06-12 02:30:12.000000000 +0200 +++ new/kargo-cli-1.10.8/ui/src/features/settings/cluster-config/cluster-config.tsx 2026-06-26 01:39:10.000000000 +0200 @@ -14,7 +14,7 @@ import { FieldContainer } from '@ui/features/common/form/field-container'; import { useModal } from '@ui/features/common/modal/use-modal'; import { Webhooks } from '@ui/features/project/settings/views/project-config/webhooks'; -import { useUpdateResource } from '@ui/gen/api/v2/resources/resources'; +import { useCreateResource, useUpdateResource } from '@ui/gen/api/v2/resources/resources'; import { useGetClusterConfig } from '@ui/gen/api/v2/system/system'; import clusterConfigSchema from '@ui/gen/schema/clusterconfigs.kargo.akuity.io_v1alpha1.json'; import { zodValidators } from '@ui/utils/validators'; @@ -75,7 +75,7 @@ resolver: zodResolver(formSchema) }); - const updateResourceMutation = useUpdateResource({ + const mutationOptions = { mutation: { onSuccess: () => { message.success({ @@ -84,14 +84,19 @@ getClusterConfigQuery.refetch(); } } - }); + }; + + const createMutation = useCreateResource(mutationOptions); + const updateMutation = useUpdateResource(mutationOptions); + + const createOrUpdateMutation = creation ? createMutation : updateMutation; const createWebhookModal = useModal((props) => ( <CreateWebhookModal clusterConfigYAML={clusterConfigYAML} project={name || ''} {...props} /> )); const onSubmitConfig = clusterConfigForm.handleSubmit((data) => - updateResourceMutation.mutate({ data: data.value }) + createOrUpdateMutation.mutate({ data: data.value }) ); return ( @@ -121,7 +126,7 @@ className='ml-auto' type='primary' onClick={onSubmitConfig} - loading={updateResourceMutation.isPending} + loading={createOrUpdateMutation.isPending} > {creation ? 'Create' : 'Update'} </Button> ++++++ kargo-cli.obsinfo ++++++ --- /var/tmp/diff_new_pack.KAIf8x/_old 2026-07-01 16:52:22.561863278 +0200 +++ /var/tmp/diff_new_pack.KAIf8x/_new 2026-07-01 16:52:22.565863417 +0200 @@ -1,5 +1,5 @@ name: kargo-cli -version: 1.10.7 -mtime: 1781224212 -commit: 17077e406ed309fcc830d08776d9b9a47fe6e134 +version: 1.10.8 +mtime: 1782430750 +commit: 50d29c10153965114f903645c83eadbe7bf58d3f ++++++ vendor.tar.gz ++++++ /work/SRC/openSUSE:Factory/kargo-cli/vendor.tar.gz /work/SRC/openSUSE:Factory/.kargo-cli.new.11887/vendor.tar.gz differ: char 13, line 1
