Browse Source

feat: add source null byte policy (#6194)

* feat: add target null byte policy

Signed-off-by: Lucas Alves <lucasalves@tremendous.com>

* fix: also consider immutable secrets

Signed-off-by: Lucas Alves <lucasalves@tremendous.com>

* fix: code quality reviews

Signed-off-by: Lucas Alves <lucasalves@tremendous.com>

* feat: move config to be source-scoped

Signed-off-by: Lucas Alves <lucasalves@tremendous.com>

* test: update generated api artifacts

Signed-off-by: Lucas Alves <lucasalves@tremendous.com>

---------

Signed-off-by: Lucas Alves <lucasalves@tremendous.com>
Lucas Severo Alves 5 days ago
parent
commit
b3e8511230

+ 22 - 0
apis/externalsecrets/v1/externalsecret_types.go

@@ -78,6 +78,18 @@ const (
 	DeletionPolicyRetain ExternalSecretDeletionPolicy = "Retain"
 	DeletionPolicyRetain ExternalSecretDeletionPolicy = "Retain"
 )
 )
 
 
+// ExternalSecretNullBytePolicy defines how fetched secret data containing NUL bytes should be handled.
+// +kubebuilder:validation:Enum=Ignore;Fail
+type ExternalSecretNullBytePolicy string
+
+const (
+	// ExternalSecretNullBytePolicyIgnore allows fetched secret data to contain NUL bytes.
+	ExternalSecretNullBytePolicyIgnore ExternalSecretNullBytePolicy = "Ignore"
+
+	// ExternalSecretNullBytePolicyFail fails reconciliation if fetched secret data contains NUL bytes.
+	ExternalSecretNullBytePolicyFail ExternalSecretNullBytePolicy = "Fail"
+)
+
 // ExternalSecretTemplateMetadata defines metadata fields for the Secret blueprint.
 // ExternalSecretTemplateMetadata defines metadata fields for the Secret blueprint.
 type ExternalSecretTemplateMetadata struct {
 type ExternalSecretTemplateMetadata struct {
 	// +optional
 	// +optional
@@ -292,6 +304,11 @@ type ExternalSecretDataRemoteRef struct {
 	// Used to define a decoding Strategy
 	// Used to define a decoding Strategy
 	// +kubebuilder:default="None"
 	// +kubebuilder:default="None"
 	DecodingStrategy ExternalSecretDecodingStrategy `json:"decodingStrategy,omitempty"`
 	DecodingStrategy ExternalSecretDecodingStrategy `json:"decodingStrategy,omitempty"`
+
+	// +optional
+	// Controls how ESO handles fetched secret data containing NUL bytes for this source.
+	// +kubebuilder:default="Ignore"
+	NullBytePolicy ExternalSecretNullBytePolicy `json:"nullBytePolicy,omitempty"`
 }
 }
 
 
 // ExternalSecretMetadataPolicy defines policies for fetching metadata from provider secrets.
 // ExternalSecretMetadataPolicy defines policies for fetching metadata from provider secrets.
@@ -477,6 +494,11 @@ type ExternalSecretFind struct {
 	// Used to define a decoding Strategy
 	// Used to define a decoding Strategy
 	// +kubebuilder:default="None"
 	// +kubebuilder:default="None"
 	DecodingStrategy ExternalSecretDecodingStrategy `json:"decodingStrategy,omitempty"`
 	DecodingStrategy ExternalSecretDecodingStrategy `json:"decodingStrategy,omitempty"`
+
+	// +optional
+	// Controls how ESO handles fetched secret data containing NUL bytes for this find source.
+	// +kubebuilder:default="Ignore"
+	NullBytePolicy ExternalSecretNullBytePolicy `json:"nullBytePolicy,omitempty"`
 }
 }
 
 
 // FindName defines criteria for finding secrets by name patterns.
 // FindName defines criteria for finding secrets by name patterns.

+ 24 - 0
config/crds/bases/external-secrets.io_clusterexternalsecrets.yaml

@@ -119,6 +119,14 @@ spec:
                               - None
                               - None
                               - Fetch
                               - Fetch
                               type: string
                               type: string
+                            nullBytePolicy:
+                              default: Ignore
+                              description: Controls how ESO handles fetched secret
+                                data containing NUL bytes for this source.
+                              enum:
+                              - Ignore
+                              - Fail
+                              type: string
                             property:
                             property:
                               description: Used to select a specific property of the
                               description: Used to select a specific property of the
                                 Provider value (if a map), if supported
                                 Provider value (if a map), if supported
@@ -253,6 +261,14 @@ spec:
                               - None
                               - None
                               - Fetch
                               - Fetch
                               type: string
                               type: string
+                            nullBytePolicy:
+                              default: Ignore
+                              description: Controls how ESO handles fetched secret
+                                data containing NUL bytes for this source.
+                              enum:
+                              - Ignore
+                              - Fail
+                              type: string
                             property:
                             property:
                               description: Used to select a specific property of the
                               description: Used to select a specific property of the
                                 Provider value (if a map), if supported
                                 Provider value (if a map), if supported
@@ -292,6 +308,14 @@ spec:
                                   description: Finds secrets base
                                   description: Finds secrets base
                                   type: string
                                   type: string
                               type: object
                               type: object
+                            nullBytePolicy:
+                              default: Ignore
+                              description: Controls how ESO handles fetched secret
+                                data containing NUL bytes for this find source.
+                              enum:
+                              - Ignore
+                              - Fail
+                              type: string
                             path:
                             path:
                               description: A root path to start the find operations.
                               description: A root path to start the find operations.
                               type: string
                               type: string

+ 24 - 0
config/crds/bases/external-secrets.io_externalsecrets.yaml

@@ -105,6 +105,14 @@ spec:
                           - None
                           - None
                           - Fetch
                           - Fetch
                           type: string
                           type: string
+                        nullBytePolicy:
+                          default: Ignore
+                          description: Controls how ESO handles fetched secret data
+                            containing NUL bytes for this source.
+                          enum:
+                          - Ignore
+                          - Fail
+                          type: string
                         property:
                         property:
                           description: Used to select a specific property of the Provider
                           description: Used to select a specific property of the Provider
                             value (if a map), if supported
                             value (if a map), if supported
@@ -238,6 +246,14 @@ spec:
                           - None
                           - None
                           - Fetch
                           - Fetch
                           type: string
                           type: string
+                        nullBytePolicy:
+                          default: Ignore
+                          description: Controls how ESO handles fetched secret data
+                            containing NUL bytes for this source.
+                          enum:
+                          - Ignore
+                          - Fail
+                          type: string
                         property:
                         property:
                           description: Used to select a specific property of the Provider
                           description: Used to select a specific property of the Provider
                             value (if a map), if supported
                             value (if a map), if supported
@@ -277,6 +293,14 @@ spec:
                               description: Finds secrets base
                               description: Finds secrets base
                               type: string
                               type: string
                           type: object
                           type: object
+                        nullBytePolicy:
+                          default: Ignore
+                          description: Controls how ESO handles fetched secret data
+                            containing NUL bytes for this find source.
+                          enum:
+                          - Ignore
+                          - Fail
+                          type: string
                         path:
                         path:
                           description: A root path to start the find operations.
                           description: A root path to start the find operations.
                           type: string
                           type: string

+ 42 - 0
deploy/crds/bundle.yaml

@@ -113,6 +113,13 @@ spec:
                                   - None
                                   - None
                                   - Fetch
                                   - Fetch
                                 type: string
                                 type: string
+                              nullBytePolicy:
+                                default: Ignore
+                                description: Controls how ESO handles fetched secret data containing NUL bytes for this source.
+                                enum:
+                                  - Ignore
+                                  - Fail
+                                type: string
                               property:
                               property:
                                 description: Used to select a specific property of the Provider value (if a map), if supported
                                 description: Used to select a specific property of the Provider value (if a map), if supported
                                 type: string
                                 type: string
@@ -240,6 +247,13 @@ spec:
                                   - None
                                   - None
                                   - Fetch
                                   - Fetch
                                 type: string
                                 type: string
+                              nullBytePolicy:
+                                default: Ignore
+                                description: Controls how ESO handles fetched secret data containing NUL bytes for this source.
+                                enum:
+                                  - Ignore
+                                  - Fail
+                                type: string
                               property:
                               property:
                                 description: Used to select a specific property of the Provider value (if a map), if supported
                                 description: Used to select a specific property of the Provider value (if a map), if supported
                                 type: string
                                 type: string
@@ -277,6 +291,13 @@ spec:
                                     description: Finds secrets base
                                     description: Finds secrets base
                                     type: string
                                     type: string
                                 type: object
                                 type: object
+                              nullBytePolicy:
+                                default: Ignore
+                                description: Controls how ESO handles fetched secret data containing NUL bytes for this find source.
+                                enum:
+                                  - Ignore
+                                  - Fail
+                                type: string
                               path:
                               path:
                                 description: A root path to start the find operations.
                                 description: A root path to start the find operations.
                                 type: string
                                 type: string
@@ -12442,6 +12463,13 @@ spec:
                               - None
                               - None
                               - Fetch
                               - Fetch
                             type: string
                             type: string
+                          nullBytePolicy:
+                            default: Ignore
+                            description: Controls how ESO handles fetched secret data containing NUL bytes for this source.
+                            enum:
+                              - Ignore
+                              - Fail
+                            type: string
                           property:
                           property:
                             description: Used to select a specific property of the Provider value (if a map), if supported
                             description: Used to select a specific property of the Provider value (if a map), if supported
                             type: string
                             type: string
@@ -12569,6 +12597,13 @@ spec:
                               - None
                               - None
                               - Fetch
                               - Fetch
                             type: string
                             type: string
+                          nullBytePolicy:
+                            default: Ignore
+                            description: Controls how ESO handles fetched secret data containing NUL bytes for this source.
+                            enum:
+                              - Ignore
+                              - Fail
+                            type: string
                           property:
                           property:
                             description: Used to select a specific property of the Provider value (if a map), if supported
                             description: Used to select a specific property of the Provider value (if a map), if supported
                             type: string
                             type: string
@@ -12606,6 +12641,13 @@ spec:
                                 description: Finds secrets base
                                 description: Finds secrets base
                                 type: string
                                 type: string
                             type: object
                             type: object
+                          nullBytePolicy:
+                            default: Ignore
+                            description: Controls how ESO handles fetched secret data containing NUL bytes for this find source.
+                            enum:
+                              - Ignore
+                              - Fail
+                            type: string
                           path:
                           path:
                             description: A root path to start the find operations.
                             description: A root path to start the find operations.
                             type: string
                             type: string

+ 53 - 0
docs/api/spec.md

@@ -4081,6 +4081,20 @@ ExternalSecretDecodingStrategy
 <p>Used to define a decoding Strategy</p>
 <p>Used to define a decoding Strategy</p>
 </td>
 </td>
 </tr>
 </tr>
+<tr>
+<td>
+<code>nullBytePolicy</code></br>
+<em>
+<a href="#external-secrets.io/v1.ExternalSecretNullBytePolicy">
+ExternalSecretNullBytePolicy
+</a>
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>Controls how ESO handles fetched secret data containing NUL bytes for this source.</p>
+</td>
+</tr>
 </tbody>
 </tbody>
 </table>
 </table>
 <h3 id="external-secrets.io/v1.ExternalSecretDecodingStrategy">ExternalSecretDecodingStrategy
 <h3 id="external-secrets.io/v1.ExternalSecretDecodingStrategy">ExternalSecretDecodingStrategy
@@ -4232,6 +4246,20 @@ ExternalSecretDecodingStrategy
 <p>Used to define a decoding Strategy</p>
 <p>Used to define a decoding Strategy</p>
 </td>
 </td>
 </tr>
 </tr>
+<tr>
+<td>
+<code>nullBytePolicy</code></br>
+<em>
+<a href="#external-secrets.io/v1.ExternalSecretNullBytePolicy">
+ExternalSecretNullBytePolicy
+</a>
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>Controls how ESO handles fetched secret data containing NUL bytes for this find source.</p>
+</td>
+</tr>
 </tbody>
 </tbody>
 </table>
 </table>
 <h3 id="external-secrets.io/v1.ExternalSecretMetadata">ExternalSecretMetadata
 <h3 id="external-secrets.io/v1.ExternalSecretMetadata">ExternalSecretMetadata
@@ -4299,6 +4327,31 @@ map[string]string
 </td>
 </td>
 </tr></tbody>
 </tr></tbody>
 </table>
 </table>
+<h3 id="external-secrets.io/v1.ExternalSecretNullBytePolicy">ExternalSecretNullBytePolicy
+(<code>string</code> alias)</p></h3>
+<p>
+(<em>Appears on:</em>
+<a href="#external-secrets.io/v1.ExternalSecretDataRemoteRef">ExternalSecretDataRemoteRef</a>, 
+<a href="#external-secrets.io/v1.ExternalSecretFind">ExternalSecretFind</a>)
+</p>
+<p>
+<p>ExternalSecretNullBytePolicy defines how fetched secret data containing NUL bytes should be handled.</p>
+</p>
+<table>
+<thead>
+<tr>
+<th>Value</th>
+<th>Description</th>
+</tr>
+</thead>
+<tbody><tr><td><p>&#34;Fail&#34;</p></td>
+<td><p>ExternalSecretNullBytePolicyFail fails reconciliation if fetched secret data contains NUL bytes.</p>
+</td>
+</tr><tr><td><p>&#34;Ignore&#34;</p></td>
+<td><p>ExternalSecretNullBytePolicyIgnore allows fetched secret data to contain NUL bytes.</p>
+</td>
+</tr></tbody>
+</table>
 <h3 id="external-secrets.io/v1.ExternalSecretRefreshPolicy">ExternalSecretRefreshPolicy
 <h3 id="external-secrets.io/v1.ExternalSecretRefreshPolicy">ExternalSecretRefreshPolicy
 (<code>string</code> alias)</p></h3>
 (<code>string</code> alias)</p></h3>
 <p>
 <p>

+ 2 - 1
pkg/controllers/clusterexternalsecret/clusterexternalsecret_controller_test.go

@@ -83,7 +83,8 @@ var _ = Describe("ClusterExternalSecret controller", func() {
 						{
 						{
 							SecretKey: "test-secret-key",
 							SecretKey: "test-secret-key",
 							RemoteRef: esv1.ExternalSecretDataRemoteRef{
 							RemoteRef: esv1.ExternalSecretDataRemoteRef{
-								Key: "test-remote-key",
+								Key:            "test-remote-key",
+								NullBytePolicy: esv1.ExternalSecretNullBytePolicyIgnore,
 							},
 							},
 						},
 						},
 					},
 					},

+ 39 - 0
pkg/controllers/externalsecret/externalsecret_controller_secret.go

@@ -17,10 +17,12 @@ limitations under the License.
 package externalsecret
 package externalsecret
 
 
 import (
 import (
+	"bytes"
 	"context"
 	"context"
 	"encoding/json"
 	"encoding/json"
 	"errors"
 	"errors"
 	"fmt"
 	"fmt"
+	"slices"
 	"strconv"
 	"strconv"
 
 
 	v1 "k8s.io/api/core/v1"
 	v1 "k8s.io/api/core/v1"
@@ -136,6 +138,9 @@ func (r *Reconciler) handleSecretData(ctx context.Context, externalSecret *esv1.
 	if err != nil {
 	if err != nil {
 		return fmt.Errorf(errDecode, secretRef.RemoteRef.DecodingStrategy, err)
 		return fmt.Errorf(errDecode, secretRef.RemoteRef.DecodingStrategy, err)
 	}
 	}
+	if err := validateFetchedSecretValue(secretRef.RemoteRef.NullBytePolicy, secretRef.SecretKey, secretData); err != nil {
+		return err
+	}
 
 
 	// store the secret data
 	// store the secret data
 	providerData[secretRef.SecretKey] = secretData
 	providerData[secretRef.SecretKey] = secretData
@@ -246,6 +251,9 @@ func (r *Reconciler) handleExtractSecrets(
 	if err != nil {
 	if err != nil {
 		return nil, fmt.Errorf(errDecode, remoteRef.Extract.DecodingStrategy, err)
 		return nil, fmt.Errorf(errDecode, remoteRef.Extract.DecodingStrategy, err)
 	}
 	}
+	if err := validateFetchedSecretMap(remoteRef.Extract.NullBytePolicy, secretMap); err != nil {
+		return nil, err
+	}
 	if genState != nil {
 	if genState != nil {
 		genState.EnqueueFlagLatestStateForGC(generatorStateKey(i))
 		genState.EnqueueFlagLatestStateForGC(generatorStateKey(i))
 	}
 	}
@@ -294,12 +302,43 @@ func (r *Reconciler) handleFindAllSecrets(
 	if err != nil {
 	if err != nil {
 		return nil, fmt.Errorf(errDecode, remoteRef.Find.DecodingStrategy, err)
 		return nil, fmt.Errorf(errDecode, remoteRef.Find.DecodingStrategy, err)
 	}
 	}
+	if err := validateFetchedSecretMap(remoteRef.Find.NullBytePolicy, secretMap); err != nil {
+		return nil, err
+	}
 	if genState != nil {
 	if genState != nil {
 		genState.EnqueueFlagLatestStateForGC(generatorStateKey(i))
 		genState.EnqueueFlagLatestStateForGC(generatorStateKey(i))
 	}
 	}
 	return secretMap, nil
 	return secretMap, nil
 }
 }
 
 
+func validateFetchedSecretValue(policy esv1.ExternalSecretNullBytePolicy, key string, value []byte) error {
+	if policy != esv1.ExternalSecretNullBytePolicyFail || !bytes.Contains(value, []byte{0}) {
+		return nil
+	}
+
+	return fmt.Errorf(`fetched secret value for key %q contains null bytes; use a text-safe format such as PEM/base64 or leave nullBytePolicy unset for intentional binary data`, key)
+}
+
+func validateFetchedSecretMap(policy esv1.ExternalSecretNullBytePolicy, secretMap map[string][]byte) error {
+	if policy != esv1.ExternalSecretNullBytePolicyFail {
+		return nil
+	}
+
+	keys := make([]string, 0, len(secretMap))
+	for key := range secretMap {
+		keys = append(keys, key)
+	}
+	slices.Sort(keys)
+
+	for _, key := range keys {
+		if err := validateFetchedSecretValue(policy, key, secretMap[key]); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 func shouldSkipGenerator(r *Reconciler, generatorDef *apiextensions.JSON) (bool, error) {
 func shouldSkipGenerator(r *Reconciler, generatorDef *apiextensions.JSON) (bool, error) {
 	var genControllerClass genv1alpha1.ControllerClassResource
 	var genControllerClass genv1alpha1.ControllerClassResource
 	err := json.Unmarshal(generatorDef.Raw, &genControllerClass)
 	err := json.Unmarshal(generatorDef.Raw, &genControllerClass)

+ 89 - 0
pkg/controllers/externalsecret/externalsecret_controller_test.go

@@ -58,6 +58,7 @@ const (
 	annotationValue    = "annotation-value"
 	annotationValue    = "annotation-value"
 	existingLabelKey   = "existing-label-key"
 	existingLabelKey   = "existing-label-key"
 	existingLabelValue = "existing-label-value"
 	existingLabelValue = "existing-label-value"
+	nullByteSecretVal  = "A\x00B"
 )
 )
 
 
 var (
 var (
@@ -2125,6 +2126,91 @@ var _ = Describe("ExternalSecret controller", Serial, func() {
 			Expect(string(secret.Data[targetProp])).To(Equal(secretVal))
 			Expect(string(secret.Data[targetProp])).To(Equal(secretVal))
 		}
 		}
 	}
 	}
+	failWhenFetchedSecretContainsNullBytes := func(tc *testCase) {
+		tc.externalSecret.Spec.Data[0].RemoteRef.NullBytePolicy = esv1.ExternalSecretNullBytePolicyFail
+		fakeProvider.WithGetSecret([]byte(nullByteSecretVal), nil)
+		tc.checkCondition = func(es *esv1.ExternalSecret) bool {
+			cond := GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
+			if cond == nil || cond.Status != v1.ConditionFalse || cond.Reason != esv1.ConditionReasonSecretSyncedError {
+				return false
+			}
+			return true
+		}
+		tc.checkExternalSecret = func(_ *esv1.ExternalSecret) {
+			secretLookupKey := types.NamespacedName{
+				Name:      tc.targetSecretName,
+				Namespace: ExternalSecretNamespace,
+			}
+			Consistently(func() bool {
+				err := k8sClient.Get(context.Background(), secretLookupKey, &v1.Secret{})
+				return apierrors.IsNotFound(err)
+			}, time.Second, interval).Should(BeTrue())
+		}
+	}
+	failWhenExtractedSecretContainsNullBytes := func(tc *testCase) {
+		tc.externalSecret.Spec.Data = nil
+		tc.externalSecret.Spec.DataFrom = []esv1.ExternalSecretDataFromRemoteRef{
+			{
+				Extract: &esv1.ExternalSecretDataRemoteRef{
+					Key:            remoteKey,
+					NullBytePolicy: esv1.ExternalSecretNullBytePolicyFail,
+				},
+			},
+		}
+		fakeProvider.WithGetSecretMap(map[string][]byte{
+			"payload": []byte(nullByteSecretVal),
+		}, nil)
+		tc.checkCondition = func(es *esv1.ExternalSecret) bool {
+			cond := GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
+			if cond == nil || cond.Status != v1.ConditionFalse || cond.Reason != esv1.ConditionReasonSecretSyncedError {
+				return false
+			}
+			return true
+		}
+		tc.checkExternalSecret = func(_ *esv1.ExternalSecret) {
+			secretLookupKey := types.NamespacedName{
+				Name:      tc.targetSecretName,
+				Namespace: ExternalSecretNamespace,
+			}
+			Consistently(func() bool {
+				err := k8sClient.Get(context.Background(), secretLookupKey, &v1.Secret{})
+				return apierrors.IsNotFound(err)
+			}, time.Second, interval).Should(BeTrue())
+		}
+	}
+	failWhenFoundSecretContainsNullBytes := func(tc *testCase) {
+		tc.externalSecret.Spec.Data = nil
+		tc.externalSecret.Spec.DataFrom = []esv1.ExternalSecretDataFromRemoteRef{
+			{
+				Find: &esv1.ExternalSecretFind{
+					Name: &esv1.FindName{
+						RegExp: "foobar",
+					},
+					NullBytePolicy: esv1.ExternalSecretNullBytePolicyFail,
+				},
+			},
+		}
+		fakeProvider.WithGetAllSecrets(map[string][]byte{
+			"payload": []byte(nullByteSecretVal),
+		}, nil)
+		tc.checkCondition = func(es *esv1.ExternalSecret) bool {
+			cond := GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
+			if cond == nil || cond.Status != v1.ConditionFalse || cond.Reason != esv1.ConditionReasonSecretSyncedError {
+				return false
+			}
+			return true
+		}
+		tc.checkExternalSecret = func(_ *esv1.ExternalSecret) {
+			secretLookupKey := types.NamespacedName{
+				Name:      tc.targetSecretName,
+				Namespace: ExternalSecretNamespace,
+			}
+			Consistently(func() bool {
+				err := k8sClient.Get(context.Background(), secretLookupKey, &v1.Secret{})
+				return apierrors.IsNotFound(err)
+			}, time.Second, interval).Should(BeTrue())
+		}
+	}
 	useClusterSecretStore := func(tc *testCase) {
 	useClusterSecretStore := func(tc *testCase) {
 		tc.secretStore = &esv1.ClusterSecretStore{
 		tc.secretStore = &esv1.ClusterSecretStore{
 			ObjectMeta: metav1.ObjectMeta{
 			ObjectMeta: metav1.ObjectMeta{
@@ -2398,6 +2484,9 @@ var _ = Describe("ExternalSecret controller", Serial, func() {
 		Entry("should sync template with correct value precedence", syncWithTemplatePrecedence),
 		Entry("should sync template with correct value precedence", syncWithTemplatePrecedence),
 		Entry("should sync template from keys and values", syncTemplateFromKeysAndValues),
 		Entry("should sync template from keys and values", syncTemplateFromKeysAndValues),
 		Entry("should sync template from literal", syncTemplateFromLiteral),
 		Entry("should sync template from literal", syncTemplateFromLiteral),
+		Entry("should fail when fetched secret data contains null bytes and remoteRef.nullBytePolicy=Fail", failWhenFetchedSecretContainsNullBytes),
+		Entry("should fail when extracted secret data contains null bytes and extract.nullBytePolicy=Fail", failWhenExtractedSecretContainsNullBytes),
+		Entry("should fail when found secret data contains null bytes and find.nullBytePolicy=Fail", failWhenFoundSecretContainsNullBytes),
 		Entry("should update template if ExternalSecret is updated", templateShouldRewrite),
 		Entry("should update template if ExternalSecret is updated", templateShouldRewrite),
 		Entry("should keep data with templates if MergePolicy=Merge", templateShouldMerge),
 		Entry("should keep data with templates if MergePolicy=Merge", templateShouldMerge),
 		Entry("should refresh secret from template", refreshWithTemplate),
 		Entry("should refresh secret from template", refreshWithTemplate),

+ 126 - 0
pkg/controllers/externalsecret/externalsecret_controller_validation_test.go

@@ -0,0 +1,126 @@
+/*
+Copyright © The ESO Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    https://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package externalsecret
+
+import (
+	"strings"
+	"testing"
+
+	esv1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1"
+)
+
+func TestValidateFetchedSecretValue(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name    string
+		policy  esv1.ExternalSecretNullBytePolicy
+		key     string
+		value   []byte
+		wantErr string
+	}{
+		{
+			name:   "zero value policy behaves like ignore",
+			policy: "",
+			key:    "payload",
+			value:  []byte(nullByteSecretVal),
+		},
+		{
+			name:   "ignores null bytes when policy is not fail",
+			policy: esv1.ExternalSecretNullBytePolicyIgnore,
+			key:    "payload",
+			value:  []byte(nullByteSecretVal),
+		},
+		{
+			name:   "allows nil values",
+			policy: esv1.ExternalSecretNullBytePolicyFail,
+			key:    "payload",
+			value:  nil,
+		},
+		{
+			name:   "allows fetched data without null bytes",
+			policy: esv1.ExternalSecretNullBytePolicyFail,
+			key:    "payload",
+			value:  []byte("QQBC"),
+		},
+		{
+			name:    "fails on fetched data containing null bytes",
+			policy:  esv1.ExternalSecretNullBytePolicyFail,
+			key:     "payload",
+			value:   []byte(nullByteSecretVal),
+			wantErr: `fetched secret value for key "payload" contains null bytes`,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			assertFetchedSecretValidationError(t, validateFetchedSecretValue(tt.policy, tt.key, tt.value), tt.wantErr)
+		})
+	}
+}
+
+func TestValidateFetchedSecretMap(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name    string
+		policy  esv1.ExternalSecretNullBytePolicy
+		data    map[string][]byte
+		wantErr string
+	}{
+		{
+			name:   "allows nil secret data map",
+			policy: esv1.ExternalSecretNullBytePolicyFail,
+			data:   nil,
+		},
+		{
+			name:   "reports the first offending key in sorted order",
+			policy: esv1.ExternalSecretNullBytePolicyFail,
+			data: map[string][]byte{
+				"zeta":  []byte(nullByteSecretVal),
+				"alpha": []byte("C\x00D"),
+			},
+			wantErr: `fetched secret value for key "alpha" contains null bytes`,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			assertFetchedSecretValidationError(t, validateFetchedSecretMap(tt.policy, tt.data), tt.wantErr)
+		})
+	}
+}
+
+func assertFetchedSecretValidationError(t *testing.T, err error, wantErr string) {
+	t.Helper()
+
+	if wantErr == "" {
+		if err != nil {
+			t.Fatalf("unexpected error = %v", err)
+		}
+		return
+	}
+
+	if err == nil {
+		t.Fatalf("error = nil, want substring %q", wantErr)
+	}
+	if got := err.Error(); !strings.Contains(got, wantErr) {
+		t.Fatalf("error = %q, want substring %q", got, wantErr)
+	}
+}

+ 3 - 0
tests/__snapshot__/clusterexternalsecret-v1.yaml

@@ -13,6 +13,7 @@ spec:
         decodingStrategy: "None"
         decodingStrategy: "None"
         key: string
         key: string
         metadataPolicy: "None"
         metadataPolicy: "None"
+        nullBytePolicy: "Ignore"
         property: string
         property: string
         version: string
         version: string
       secretKey: string
       secretKey: string
@@ -30,6 +31,7 @@ spec:
         decodingStrategy: "None"
         decodingStrategy: "None"
         key: string
         key: string
         metadataPolicy: "None"
         metadataPolicy: "None"
+        nullBytePolicy: "Ignore"
         property: string
         property: string
         version: string
         version: string
       find:
       find:
@@ -37,6 +39,7 @@ spec:
         decodingStrategy: "None"
         decodingStrategy: "None"
         name:
         name:
           regexp: string
           regexp: string
+        nullBytePolicy: "Ignore"
         path: string
         path: string
         tags: {}
         tags: {}
       rewrite:
       rewrite:

+ 3 - 0
tests/__snapshot__/externalsecret-v1.yaml

@@ -8,6 +8,7 @@ spec:
       decodingStrategy: "None"
       decodingStrategy: "None"
       key: string
       key: string
       metadataPolicy: "None"
       metadataPolicy: "None"
+      nullBytePolicy: "Ignore"
       property: string
       property: string
       version: string
       version: string
     secretKey: string
     secretKey: string
@@ -25,6 +26,7 @@ spec:
       decodingStrategy: "None"
       decodingStrategy: "None"
       key: string
       key: string
       metadataPolicy: "None"
       metadataPolicy: "None"
+      nullBytePolicy: "Ignore"
       property: string
       property: string
       version: string
       version: string
     find:
     find:
@@ -32,6 +34,7 @@ spec:
       decodingStrategy: "None"
       decodingStrategy: "None"
       name:
       name:
         regexp: string
         regexp: string
+      nullBytePolicy: "Ignore"
       path: string
       path: string
       tags: {}
       tags: {}
     rewrite:
     rewrite: