From d28594d2f97bcd148f31f4661d4febc8e18ceb80 Mon Sep 17 00:00:00 2001 From: Dana Axinte <53751979+dana-axinte@users.noreply.github.com> Date: Mon, 30 Jun 2025 22:28:41 +0100 Subject: [PATCH] SecretsManager: Limit of 24kiB for raw input for secure value (#107403) * SecureValues: Add limit of 24kiB for raw input Co-authored-by: Matheus Macabu * Fix lint --------- Co-authored-by: Matheus Macabu --- pkg/apis/secret/v0alpha1/secure_value.go | 2 ++ pkg/apis/secret/v0alpha1/zz_generated.openapi.go | 3 ++- pkg/registry/apis/secret/contracts/secure_value.go | 3 +++ .../apis/secret/reststorage/secure_value_rest.go | 7 +++++++ .../secret/reststorage/secure_value_rest_test.go | 12 ++++++++++++ 5 files changed, 26 insertions(+), 1 deletion(-) diff --git a/pkg/apis/secret/v0alpha1/secure_value.go b/pkg/apis/secret/v0alpha1/secure_value.go index 86212f39c02..26a68263568 100644 --- a/pkg/apis/secret/v0alpha1/secure_value.go +++ b/pkg/apis/secret/v0alpha1/secure_value.go @@ -59,7 +59,9 @@ type SecureValueSpec struct { // The raw value is only valid for write. Read/List will always be empty. // There is no support for mixing `value` and `ref`, you can't create a secret in a third-party keeper with a specified `ref`. + // Minimum and maximum lengths in bytes. // +k8s:validation:minLength=1 + // +k8s:validation:maxLength=24576 Value ExposedSecureValue `json:"value,omitempty"` // When using a third-party keeper, the `ref` is used to reference a value inside the remote storage. diff --git a/pkg/apis/secret/v0alpha1/zz_generated.openapi.go b/pkg/apis/secret/v0alpha1/zz_generated.openapi.go index 836db012cb0..9bb5f4753e5 100644 --- a/pkg/apis/secret/v0alpha1/zz_generated.openapi.go +++ b/pkg/apis/secret/v0alpha1/zz_generated.openapi.go @@ -641,8 +641,9 @@ func schema_pkg_apis_secret_v0alpha1_SecureValueSpec(ref common.ReferenceCallbac }, "value": { SchemaProps: spec.SchemaProps{ - Description: "The raw value is only valid for write. Read/List will always be empty. There is no support for mixing `value` and `ref`, you can't create a secret in a third-party keeper with a specified `ref`.", + Description: "The raw value is only valid for write. Read/List will always be empty. There is no support for mixing `value` and `ref`, you can't create a secret in a third-party keeper with a specified `ref`. Minimum and maximum lengths in bytes.", MinLength: ptr.To[int64](1), + MaxLength: ptr.To[int64](24576), Type: []string{"string"}, Format: "", }, diff --git a/pkg/registry/apis/secret/contracts/secure_value.go b/pkg/registry/apis/secret/contracts/secure_value.go index 25508f16569..ca3f66f991d 100644 --- a/pkg/registry/apis/secret/contracts/secure_value.go +++ b/pkg/registry/apis/secret/contracts/secure_value.go @@ -8,6 +8,9 @@ import ( "github.com/grafana/grafana/pkg/registry/apis/secret/xkube" ) +// The maximum size of a secure value in bytes when written as raw input. +const SECURE_VALUE_RAW_INPUT_MAX_SIZE_BYTES = 24576 // 24 KiB + type DecryptSecureValue struct { Keeper *string Ref string diff --git a/pkg/registry/apis/secret/reststorage/secure_value_rest.go b/pkg/registry/apis/secret/reststorage/secure_value_rest.go index 824a77869f7..0aed12aac53 100644 --- a/pkg/registry/apis/secret/reststorage/secure_value_rest.go +++ b/pkg/registry/apis/secret/reststorage/secure_value_rest.go @@ -245,6 +245,13 @@ func ValidateSecureValue(sv, oldSv *secretv0alpha1.SecureValue, operation admiss } // General validations. + if len(sv.Spec.Value) > contracts.SECURE_VALUE_RAW_INPUT_MAX_SIZE_BYTES { + errs = append( + errs, + field.TooLong(field.NewPath("spec", "value"), len(sv.Spec.Value), contracts.SECURE_VALUE_RAW_INPUT_MAX_SIZE_BYTES), + ) + } + if errs := validateDecrypters(sv.Spec.Decrypters, decryptersAllowList); len(errs) > 0 { return errs } diff --git a/pkg/registry/apis/secret/reststorage/secure_value_rest_test.go b/pkg/registry/apis/secret/reststorage/secure_value_rest_test.go index 113faf6f0aa..1cde8d63816 100644 --- a/pkg/registry/apis/secret/reststorage/secure_value_rest_test.go +++ b/pkg/registry/apis/secret/reststorage/secure_value_rest_test.go @@ -4,12 +4,14 @@ import ( "fmt" "maps" "slices" + "strings" "testing" "github.com/stretchr/testify/require" "k8s.io/apiserver/pkg/admission" secretv0alpha1 "github.com/grafana/grafana/pkg/apis/secret/v0alpha1" + "github.com/grafana/grafana/pkg/registry/apis/secret/contracts" ) func TestValidateSecureValue(t *testing.T) { @@ -50,6 +52,16 @@ func TestValidateSecureValue(t *testing.T) { require.Len(t, errs, 1) require.Equal(t, "spec", errs[0].Field) }) + + t.Run("`value` cannot exceed 24576 bytes", func(t *testing.T) { + sv := validSecureValue.DeepCopy() + sv.Spec.Value = secretv0alpha1.NewExposedSecureValue(strings.Repeat("a", contracts.SECURE_VALUE_RAW_INPUT_MAX_SIZE_BYTES+1)) + sv.Spec.Ref = nil + + errs := ValidateSecureValue(sv, nil, admission.Create, nil) + require.Len(t, errs, 1) + require.Equal(t, "spec.value", errs[0].Field) + }) }) t.Run("when updating a securevalue", func(t *testing.T) {