feat(core): report safe reconcile errors in status conditions (#6834)

* feat(core): report safe reconcile errors in status conditions

Reconciliation failures replaced the underlying error with a fixed string before writing
status.conditions, so diagnosing a store or an ExternalSecret meant reaching for controller
logs.

Error text is now published only when explicitly marked with ctrlutil.Safe, applied to
errors ESO constructs itself and to Kubernetes API errors. Provider and template errors stay
generic on purpose: #5884 showed a provider unmarshal error can echo secret material into a
condition, and template rendering can echo rendered values. SafeMessage resolves the
innermost marker, so text composed around a marked error is never published, and caps the
result at 256 runes because the CRDs set no maxLength on Message.

Store provider resolution now reports ProviderNotFound separately from a client that failed
to build, and the immutable and owned-by-other dead ends get their own reasons rather than a
shared SecretSyncedError.

Raw provider error text, item 3 of the issue, is deliberately not included here.

Refs: external-secrets/external-secrets#6832
Signed-off-by: Alexander Chernov <alexander@chernov.it>

* fix(core): keep truncated safe errors within the rune cap

truncate appended the "..." marker after slicing to the limit, so SafeMessage
could return 259 runes where MaxConditionMessageLength is 256. Reserve the
marker's width before slicing so the constant is a real upper bound, which
matters most on the SecretStore path where the detail replaces the message
rather than being appended to it.

Refs: external-secrets/external-secrets#6832
Signed-off-by: Alexander Chernov <alexander@chernov.it>

---------

Signed-off-by: Alexander Chernov <alexander@chernov.it>
Co-authored-by: Gergely Bräutigam <gergely.brautigam@sap.com>

apis/externalsecrets/v1/externalsecret_types.go

@@ -694,6 +694,12 @@ const (
 	ConditionReasonSecretDeleted = "SecretDeleted"
 	// ConditionReasonSecretMissing indicates that the secret is missing.
 	ConditionReasonSecretMissing = "SecretMissing"
+	// ConditionReasonSecretImmutable indicates that the target secret is immutable
+	// and cannot be updated.
+	ConditionReasonSecretImmutable = "SecretImmutable"
+	// ConditionReasonSecretOwnedByOther indicates that the target secret is owned
+	// by another ExternalSecret.
+	ConditionReasonSecretOwnedByOther = "SecretOwnedByOther"
 
 	// ReasonUpdateFailed indicates that the update operation failed.
 	ReasonUpdateFailed = "UpdateFailed"

apis/externalsecrets/v1/secretstore_types.go

@@ -343,11 +343,14 @@ const (
 
 	ReasonInvalidStore          = "InvalidStoreConfiguration"
 	ReasonInvalidProviderConfig = "InvalidProviderConfig"
-	ReasonValidationFailed      = "ValidationFailed"
-	ReasonValidationUnknown     = "ValidationUnknown"
-	ReasonStoreValid            = "Valid"
-	StoreUnmaintained           = "StoreUnmaintained"
-	StoreDeprecated             = "StoreDeprecated"
+	// ReasonProviderNotFound indicates the provider named in the store spec could
+	// not be resolved, as opposed to a client that failed to be built.
+	ReasonProviderNotFound  = "ProviderNotFound"
+	ReasonValidationFailed  = "ValidationFailed"
+	ReasonValidationUnknown = "ValidationUnknown"
+	ReasonStoreValid        = "Valid"
+	StoreUnmaintained       = "StoreUnmaintained"
+	StoreDeprecated         = "StoreDeprecated"
 )
 
 // SecretStoreStatusCondition contains condition information for a SecretStore.

docs/provider/webhook.md

@@ -237,6 +237,10 @@ A `Ready` condition of `False` with reason `InvalidProviderConfig` means the cli
 not be created or that store validation failed. The accompanying event carries the
 underlying error, which is usually more specific than the condition message.
 
+Reason `ProviderNotFound` is narrower: the provider named in `spec.provider` could not be
+resolved at all, so no provider code ran. There the condition message names the backend and
+is as specific as the event.
+
 For per-secret failures, check the `ExternalSecret` instead:
 
 ```sh

pkg/controllers/externalsecret/externalsecret_controller.go

@@ -285,7 +285,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ct
 
 		// validate generic target configuration early
 		if err := r.validateGenericTarget(log, externalSecret); err != nil {
-			r.markAsFailed("invalid generic target", err, externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonSecretSyncedError)
+			r.markAsFailed("invalid generic target", ctrlutil.Safe(err), externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonSecretSyncedError)
 			return ctrl.Result{}, nil // don't requeue as this is a configuration error that is not recoverable
 		}
 
@@ -431,7 +431,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ct
 			creationPolicy := externalSecret.Spec.Target.CreationPolicy
 			if creationPolicy != esv1.CreatePolicyOwner {
 				err = fmt.Errorf(errDeleteCreatePolicy, secretName, creationPolicy)
-				r.markAsFailed(msgErrorDeleteSecret, err, externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonSecretSyncedError)
+				r.markAsFailed(msgErrorDeleteSecret, ctrlutil.Safe(err), externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonSecretSyncedError)
 				return ctrl.Result{}, nil
 			}
 
@@ -439,7 +439,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ct
 			if existingSecret.UID != "" {
 				err = r.Delete(ctx, existingSecret)
 				if err != nil && !apierrors.IsNotFound(err) {
-					r.markAsFailed(msgErrorDeleteSecret, err, externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonSecretSyncedError)
+					r.markAsFailed(msgErrorDeleteSecret, ctrlutil.Safe(err), externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonSecretSyncedError)
 					return ctrl.Result{}, err
 				}
 				log.V(1).Info(logSecretDeleted, "secret", secretName, "namespace", externalSecret.Namespace, "reason", "DeletionPolicy=Delete and provider returned no data")
@@ -539,7 +539,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ct
 		// for example, if the target secret name was changed
 		err = r.deleteOrphanedSecrets(ctx, log, externalSecret, secretName)
 		if err != nil {
-			r.markAsFailed(msgErrorDeleteOrphaned, err, externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonSecretSyncedError)
+			r.markAsFailed(msgErrorDeleteOrphaned, ctrlutil.Safe(err), externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonSecretSyncedError)
 			return ctrl.Result{}, err
 		}
 
@@ -561,24 +561,26 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ct
 		// detect errors indicating that we failed to set ourselves as the owner of the secret
 		// NOTE: this error cant be fixed by retrying so we don't return an error (which would requeue immediately)
 		if errors.Is(err, ErrSecretSetCtrlRef) {
-			r.markAsFailed(msgErrorBecomeOwner, err, externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonSecretSyncedError)
+			r.markAsFailed(msgErrorBecomeOwner, ctrlutil.Safe(err), externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonSecretSyncedError)
 			return ctrl.Result{}, nil
 		}
 
 		// detect errors indicating that the secret has another ExternalSecret as owner
 		// NOTE: this error cant be fixed by retrying so we don't return an error (which would requeue immediately)
 		if errors.Is(err, ErrSecretIsOwned) {
-			r.markAsFailed(msgErrorIsOwned, err, externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonSecretSyncedError)
+			r.markAsFailed(msgErrorIsOwned, ctrlutil.Safe(err), externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonSecretOwnedByOther)
 			return ctrl.Result{}, nil
 		}
 
 		// detect errors indicating that the secret is immutable
 		// NOTE: this error cant be fixed by retrying so we don't return an error (which would requeue immediately)
 		if errors.Is(err, ErrSecretImmutable) {
-			r.markAsFailed(msgErrorUpdateImmutable, err, externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonSecretSyncedError)
+			r.markAsFailed(msgErrorUpdateImmutable, ctrlutil.Safe(err), externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonSecretImmutable)
 			return ctrl.Result{}, nil
 		}
 
+		// not marked safe here: this path also carries template errors, which can
+		// echo rendered values. createSecret / updateSecret mark their own API errors.
 		r.markAsFailed(msgErrorUpdateSecret, err, externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonSecretSyncedError)
 		return ctrl.Result{}, err
 	}
@@ -604,7 +606,7 @@ func (r *Reconciler) reconcileGenericTarget(
 		var getErr error
 		existing, getErr = r.getGenericResource(ctx, log, externalSecret)
 		if getErr != nil && !apierrors.IsNotFound(getErr) {
-			r.markAsFailed("could not get target resource", getErr, externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonResourceSyncedError)
+			r.markAsFailed("could not get target resource", ctrlutil.Safe(getErr), externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonResourceSyncedError)
 			return ctrl.Result{}, getErr
 		}
 	}
@@ -632,13 +634,13 @@ func (r *Reconciler) reconcileGenericTarget(
 			creationPolicy := externalSecret.Spec.Target.CreationPolicy
 			if creationPolicy != esv1.CreatePolicyOwner {
 				err = fmt.Errorf("unable to delete resource: creationPolicy=%s is not Owner", creationPolicy)
-				r.markAsFailed("could not delete resource", err, externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonResourceSyncedError)
+				r.markAsFailed("could not delete resource", ctrlutil.Safe(err), externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonResourceSyncedError)
 				return ctrl.Result{}, nil
 			}
 
 			err = r.deleteGenericResource(ctx, log, externalSecret)
 			if err != nil {
-				r.markAsFailed("could not delete resource", err, externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonResourceSyncedError)
+				r.markAsFailed("could not delete resource", ctrlutil.Safe(err), externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonResourceSyncedError)
 				return ctrl.Result{}, err
 			}
 
@@ -665,6 +667,19 @@ func (r *Reconciler) reconcileGenericTarget(
 	// render the template for the manifest
 	obj, err := r.applyTemplateToManifest(ctx, externalSecret, dataMap, baseObj)
 	if err != nil {
+		// applyTemplateToManifest also applies ownership, so the same dead-end
+		// conflicts the Secret lane reports can surface here. Retrying does not
+		// fix either, hence no returned error.
+		switch {
+		case errors.Is(err, ErrSecretIsOwned):
+			r.markAsFailed(msgErrorIsOwned, ctrlutil.Safe(err), externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonSecretOwnedByOther)
+			return ctrl.Result{}, nil
+		case errors.Is(err, ErrSecretSetCtrlRef):
+			r.markAsFailed(msgErrorBecomeOwner, ctrlutil.Safe(err), externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonResourceSyncedError)
+			return ctrl.Result{}, nil
+		}
+
+		// template errors stay generic: rendering can echo secret values.
 		r.markAsFailed("could not apply template to manifest", err, externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonResourceSyncedError)
 		return ctrl.Result{}, err
 	}
@@ -704,7 +719,9 @@ func (r *Reconciler) reconcileGenericTarget(
 			return ctrl.Result{RequeueAfter: 1 * time.Second}, nil
 		}
 
-		r.markAsFailed(msgErrorUpdateSecret, err, externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonResourceSyncedError)
+		// the template was already applied above, so err here is only from the
+		// Kubernetes API call that created or updated the target resource.
+		r.markAsFailed(msgErrorUpdateSecret, ctrlutil.Safe(err), externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonResourceSyncedError)
 		return ctrl.Result{}, err
 	}
 
@@ -784,6 +801,11 @@ func (r *Reconciler) markAsDone(externalSecret *esv1.ExternalSecret, start time.
 
 func (r *Reconciler) markAsFailed(msg string, err error, externalSecret *esv1.ExternalSecret, counter prometheus.Counter, reason string) {
 	r.recorder.Event(externalSecret, v1.EventTypeWarning, esv1.ReasonUpdateFailed, err.Error())
+	// only errors explicitly marked safe are detailed here; provider errors keep
+	// the generic message because they may carry secret payloads.
+	if detail := ctrlutil.SafeMessage(err); detail != "" {
+		msg = fmt.Sprintf("%s: %s", msg, detail)
+	}
 	conditionSynced := NewExternalSecretCondition(esv1.ExternalSecretReady, v1.ConditionFalse, reason, msg)
 	SetExternalSecretCondition(externalSecret, *conditionSynced)
 	counter.Inc()
@@ -933,7 +955,7 @@ func (r *Reconciler) createSecret(ctx context.Context, mutationFunc func(secret
 
 	// note, we set field owner even for Create
 	if err := r.Create(ctx, newSecret, client.FieldOwner(fqdn)); err != nil {
-		return err
+		return ctrlutil.Safe(err)
 	}
 
 	// set the binding reference to the secret
@@ -950,7 +972,7 @@ func (r *Reconciler) updateSecret(ctx context.Context, log logr.Logger, existing
 	// fail if the secret does not exist
 	// this should never happen because we check this before calling this function
 	if existingSecret.UID == "" {
-		return fmt.Errorf(errUpdateNotFound, secretName)
+		return ctrlutil.Safe(fmt.Errorf(errUpdateNotFound, secretName))
 	}
 
 	// set the binding reference to the secret
@@ -991,7 +1013,7 @@ func (r *Reconciler) updateSecret(ctx context.Context, log logr.Logger, existing
 				if apierrors.IsConflict(err) {
 					return err
 				}
-				return fmt.Errorf(errUpdate, existingSecret.Name, err)
+				return ctrlutil.Safe(fmt.Errorf(errUpdate, existingSecret.Name, err))
 			}
 		} else {
 			// we know there was some change in the secret (or we would have returned early)
@@ -1002,7 +1024,7 @@ func (r *Reconciler) updateSecret(ctx context.Context, log logr.Logger, existing
 
 		// if the immutable data was changed, we should return an error
 		if dataChanged {
-			return fmt.Errorf(errUpdate, existingSecret.Name, ErrSecretImmutable)
+			return ctrlutil.Safe(fmt.Errorf(errUpdate, existingSecret.Name, ErrSecretImmutable))
 		}
 	}
 
@@ -1013,7 +1035,7 @@ func (r *Reconciler) updateSecret(ctx context.Context, log logr.Logger, existing
 		if apierrors.IsConflict(err) {
 			return err
 		}
-		return fmt.Errorf(errUpdate, updatedSecret.Name, err)
+		return ctrlutil.Safe(fmt.Errorf(errUpdate, updatedSecret.Name, err))
 	}
 
 	// only compute the key diff when debug verbosity is active (--loglevel=debug /

pkg/controllers/externalsecret/markasfailed_test.go

@@ -0,0 +1,122 @@
+/*
+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 (
+	"errors"
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/prometheus/client_golang/prometheus"
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/tools/record"
+
+	esv1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1"
+	ctrlutil "github.com/external-secrets/external-secrets/pkg/controllers/util"
+)
+
+// secretValue is the shape that leaked through a PushSecret condition in
+// external-secrets#5884: json.Unmarshal echoes the offending value.
+const secretValue = "8019210420527506405"
+
+func markAsFailedFixture() (*Reconciler, *esv1.ExternalSecret, prometheus.Counter) {
+	r := &Reconciler{recorder: record.NewFakeRecorder(10)}
+	es := &esv1.ExternalSecret{
+		ObjectMeta: metav1.ObjectMeta{Name: "es", Namespace: "default"},
+	}
+	return r, es, prometheus.NewCounter(prometheus.CounterOpts{Name: "test_sync_errors"})
+}
+
+func readyCondition(t *testing.T, es *esv1.ExternalSecret) *esv1.ExternalSecretStatusCondition {
+	t.Helper()
+	cond := esv1.GetExternalSecretCondition(es.Status, esv1.ExternalSecretReady)
+	if cond == nil {
+		t.Fatal("no Ready condition was set")
+	}
+	return cond
+}
+
+func TestMarkAsFailedKeepsUnsafeErrorsOutOfCondition(t *testing.T) {
+	tests := []struct {
+		name string
+		err  error
+	}{
+		{
+			name: "provider unmarshal error",
+			err:  fmt.Errorf("json: cannot unmarshal number %s into Go value of type float64", secretValue),
+		},
+		{
+			name: "template render error",
+			err:  fmt.Errorf(errApplyTemplate, fmt.Errorf("executing template: bad value %s", secretValue)),
+		},
+		{
+			name: "provider error wrapping a marked error",
+			err:  fmt.Errorf("provider said %s: %w", secretValue, ctrlutil.Safe(errors.New("connection refused"))),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			r, es, counter := markAsFailedFixture()
+
+			r.markAsFailed(msgErrorGetSecretData, tt.err, es, counter, esv1.ConditionReasonSecretSyncedError)
+
+			cond := readyCondition(t, es)
+			if strings.Contains(cond.Message, secretValue) {
+				t.Fatalf("condition message leaked provider text: %q", cond.Message)
+			}
+			if cond.Status != v1.ConditionFalse {
+				t.Errorf("status = %v, want False", cond.Status)
+			}
+		})
+	}
+}
+
+func TestMarkAsFailedDetailsSafeErrors(t *testing.T) {
+	r, es, counter := markAsFailedFixture()
+	err := ctrlutil.Safe(fmt.Errorf(errUpdate, "my-secret", ErrSecretImmutable))
+
+	r.markAsFailed(msgErrorUpdateImmutable, err, es, counter, esv1.ConditionReasonSecretImmutable)
+
+	cond := readyCondition(t, es)
+	if cond.Reason != esv1.ConditionReasonSecretImmutable {
+		t.Errorf("reason = %q, want %q", cond.Reason, esv1.ConditionReasonSecretImmutable)
+	}
+	if !strings.HasPrefix(cond.Message, msgErrorUpdateImmutable) {
+		t.Errorf("message %q lost the base text", cond.Message)
+	}
+	if !strings.Contains(cond.Message, "my-secret") {
+		t.Errorf("message %q dropped the safe detail", cond.Message)
+	}
+}
+
+// A long safe error must not grow the condition message without bound: there is
+// no maxLength on the Message field in the CRDs.
+func TestMarkAsFailedCapsMessageLength(t *testing.T) {
+	r, es, counter := markAsFailedFixture()
+	err := ctrlutil.Safe(errors.New(strings.Repeat("a", 4096)))
+
+	r.markAsFailed(msgErrorUpdateSecret, err, es, counter, esv1.ConditionReasonSecretSyncedError)
+
+	cond := readyCondition(t, es)
+	limit := len(msgErrorUpdateSecret) + len(": ") + ctrlutil.MaxConditionMessageLength
+	if len(cond.Message) > limit {
+		t.Errorf("message length = %d, want at most %d", len(cond.Message), limit)
+	}
+}

pkg/controllers/secretstore/client_manager.go

@@ -33,6 +33,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	esv1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1"
+	ctrlutil "github.com/external-secrets/external-secrets/pkg/controllers/util"
 )
 
 const (
@@ -42,6 +43,11 @@ const (
 	errClusterStoreMismatch  = "using cluster store %q is not allowed from namespace %q: denied by spec.condition"
 )
 
+// ErrProviderResolution marks a failure to resolve the provider named in the
+// store spec. It never reaches provider code, so the wrapped message is safe to
+// surface in the store status.
+var ErrProviderResolution = errors.New("could not resolve store provider")
+
 // Manager stores instances of provider clients
 // At any given time we must have no more than one instance
 // of a client (due to limitations in GCP / see mutexlock there)
@@ -84,7 +90,7 @@ func NewManager(ctrlClient client.Client, controllerClass string, enableFloodgat
 func (m *Manager) GetFromStore(ctx context.Context, store esv1.GenericStore, namespace string) (esv1.SecretsClient, error) {
 	storeProvider, err := esv1.GetProvider(store)
 	if err != nil {
-		return nil, err
+		return nil, ctrlutil.Safe(fmt.Errorf("%w: %w", ErrProviderResolution, err))
 	}
 	secretClient := m.getStoredClient(ctx, storeProvider, store)
 	if secretClient != nil {

pkg/controllers/secretstore/client_manager_saferr_test.go

@@ -0,0 +1,96 @@
+/*
+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 secretstore
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/go-logr/logr"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
+
+	esv1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1"
+	ctrlutil "github.com/external-secrets/external-secrets/pkg/controllers/util"
+)
+
+// providerLeak stands in for a provider error carrying secret material.
+const providerLeak = "auth failed for token AKIAIOSFODNN7EXAMPLE"
+
+func newSafeErrManager(t *testing.T) (*Manager, client.Client) {
+	t.Helper()
+
+	scheme := runtime.NewScheme()
+	utilruntime.Must(clientgoscheme.AddToScheme(scheme))
+	utilruntime.Must(esv1.AddToScheme(scheme))
+
+	kube := fakeclient.NewClientBuilder().WithScheme(scheme).Build()
+	return &Manager{
+		log:       logr.Discard(),
+		client:    kube,
+		clientMap: make(map[clientKey]*clientVal),
+	}, kube
+}
+
+// A store naming no provider fails before any provider code runs, so the reason
+// is reportable and the message may be surfaced in the store status.
+func TestGetFromStoreProviderResolutionIsSafe(t *testing.T) {
+	mgr, _ := newSafeErrManager(t)
+
+	store := &esv1.SecretStore{
+		ObjectMeta: metav1.ObjectMeta{Name: "no-provider", Namespace: "default"},
+		Spec:       esv1.SecretStoreSpec{Provider: &esv1.SecretStoreProvider{}},
+	}
+
+	_, err := mgr.GetFromStore(context.Background(), store, "default")
+	require.Error(t, err)
+	assert.True(t, errors.Is(err, ErrProviderResolution), "want ErrProviderResolution, got %v", err)
+	assert.Contains(t, ctrlutil.SafeMessage(err), "could not resolve store provider")
+}
+
+// A client constructor failure is provider code, so nothing may be published.
+func TestGetFromStoreClientErrorIsNotSafe(t *testing.T) {
+	mgr, _ := newSafeErrManager(t)
+
+	fakeProvider := &WrapProvider{
+		newClientFunc: func(context.Context, esv1.GenericStore, client.Client, string) (esv1.SecretsClient, error) {
+			return nil, errors.New(providerLeak)
+		},
+	}
+	esv1.ForceRegister(fakeProvider, &esv1.SecretStoreProvider{
+		AWS: &esv1.AWSProvider{},
+	}, esv1.MaintenanceStatusMaintained)
+
+	store := &esv1.SecretStore{
+		ObjectMeta: metav1.ObjectMeta{Name: "aws-store", Namespace: "default"},
+		Spec: esv1.SecretStoreSpec{
+			Provider: &esv1.SecretStoreProvider{AWS: &esv1.AWSProvider{}},
+		},
+	}
+
+	_, err := mgr.GetFromStore(context.Background(), store, "default")
+	require.Error(t, err)
+	assert.False(t, errors.Is(err, ErrProviderResolution))
+	assert.Empty(t, ctrlutil.SafeMessage(err), "provider error must not be publishable")
+}

pkg/controllers/secretstore/common.go

@@ -36,6 +36,7 @@ import (
 	esapi "github.com/external-secrets/external-secrets/apis/externalsecrets/v1"
 	esv1alpha1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1alpha1"
 	"github.com/external-secrets/external-secrets/pkg/controllers/secretstore/metrics"
+	ctrlutil "github.com/external-secrets/external-secrets/pkg/controllers/util"
 
 	// Load registered providers.
 	_ "github.com/external-secrets/external-secrets/pkg/register"
@@ -182,9 +183,15 @@ func validateStore(ctx context.Context, namespace, controllerClass string, store
 	}()
 	cl, err := mgr.GetFromStore(ctx, store, namespace)
 	if err != nil {
-		cond := NewSecretStoreCondition(esapi.SecretStoreReady, v1.ConditionFalse, esapi.ReasonInvalidProviderConfig, errUnableCreateClient)
+		// resolving the provider happens before any provider code runs, so that
+		// failure carries no remote payload and can be reported verbatim.
+		reason, msg := esapi.ReasonInvalidProviderConfig, errUnableCreateClient
+		if detail := ctrlutil.SafeMessage(err); detail != "" && errors.Is(err, ErrProviderResolution) {
+			reason, msg = esapi.ReasonProviderNotFound, detail
+		}
+		cond := NewSecretStoreCondition(esapi.SecretStoreReady, v1.ConditionFalse, reason, msg)
 		SetExternalSecretCondition(store, *cond, gaugeVecGetter)
-		recorder.Event(store, v1.EventTypeWarning, esapi.ReasonInvalidProviderConfig, err.Error())
+		recorder.Event(store, v1.EventTypeWarning, reason, err.Error())
 		return fmt.Errorf(errStoreClient, err)
 	}
 	validationResult, err := cl.Validate()

pkg/controllers/util/statuserr.go

@@ -0,0 +1,77 @@
+/*
+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 ctrlutil
+
+import "errors"
+
+// MaxConditionMessageLength caps how much of a safe error reaches a status
+// condition. The Message fields carry no maxLength in the CRDs, so a wrapped
+// chain would otherwise grow unbounded in etcd.
+const MaxConditionMessageLength = 256
+
+// safeError marks an error whose text may be copied into a status condition.
+type safeError struct {
+	err error
+}
+
+func (e *safeError) Error() string { return e.err.Error() }
+
+func (e *safeError) Unwrap() error { return e.err }
+
+// Safe marks err as publishable in a status condition. Wrap only errors ESO
+// builds itself or receives from the Kubernetes API, and never compose one from
+// text you have not vetted: marking vouches for the whole composed message.
+// Provider errors can carry secret payloads, see external-secrets#5884.
+func Safe(err error) error {
+	if err == nil {
+		return nil
+	}
+	return &safeError{err: err}
+}
+
+// SafeMessage returns the innermost marked error's text, truncated, or "" when
+// err was never marked with Safe. Innermost wins so that text composed around a
+// marked error later, by a provider or by errors.Join, is never published.
+func SafeMessage(err error) string {
+	var innermost *safeError
+	for {
+		var safe *safeError
+		if !errors.As(err, &safe) {
+			break
+		}
+		innermost = safe
+		err = safe.Unwrap()
+	}
+	if innermost == nil {
+		return ""
+	}
+	return truncate(innermost.Error(), MaxConditionMessageLength)
+}
+
+// truncate shortens msg to at most limit runes in total, counting the marker
+// that says the text was cut.
+func truncate(msg string, limit int) string {
+	const marker = "..." // ASCII, so len is both its byte and its rune count
+	runes := []rune(msg)
+	if len(runes) <= limit {
+		return msg
+	}
+	if limit < len(marker) {
+		return string(runes[:limit])
+	}
+	return string(runes[:limit-len(marker)]) + marker
+}

pkg/controllers/util/statuserr_test.go

@@ -0,0 +1,154 @@
+/*
+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 ctrlutil
+
+import (
+	"errors"
+	"fmt"
+	"strings"
+	"testing"
+)
+
+// providerErr stands in for an error returned by provider code, which may embed
+// secret material and must never reach a status condition.
+const providerErr = "json: cannot unmarshal number 8019210420527506405 into Go value of type float64"
+
+func TestSafeMessage(t *testing.T) {
+	sentinel := errors.New("secret is immutable")
+
+	tests := []struct {
+		name string
+		err  error
+		want string
+	}{
+		{
+			name: "nil error yields nothing",
+			err:  nil,
+			want: "",
+		},
+		{
+			name: "unmarked error yields nothing",
+			err:  errors.New(providerErr),
+			want: "",
+		},
+		{
+			name: "marked error is published",
+			err:  Safe(errors.New("could not update secret foo: already exists")),
+			want: "could not update secret foo: already exists",
+		},
+		{
+			name: "marking nil stays nil",
+			err:  Safe(nil),
+			want: "",
+		},
+		{
+			name: "wrapping a marked error keeps it publishable",
+			err:  fmt.Errorf("could not update secret: %w", Safe(sentinel)),
+			want: "secret is immutable",
+		},
+		{
+			name: "marked twice is not duplicated",
+			err:  Safe(Safe(errors.New("target is owned by another ExternalSecret"))),
+			want: "target is owned by another ExternalSecret",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := SafeMessage(tt.err); got != tt.want {
+				t.Errorf("SafeMessage() = %q, want %q", got, tt.want)
+			}
+		})
+	}
+}
+
+// Text composed around a marked error must never be published, whichever way it
+// was composed. Each shape here leaked before SafeMessage took the innermost mark.
+func TestSafeMessageDoesNotPublishWrapperText(t *testing.T) {
+	marked := Safe(errors.New("connection refused"))
+
+	tests := []struct {
+		name string
+		err  error
+	}{
+		{
+			name: "unmarked provider wrapper",
+			err:  fmt.Errorf("%s: %w", providerErr, marked),
+		},
+		{
+			name: "marked provider wrapper",
+			err:  Safe(fmt.Errorf("%s: %w", providerErr, marked)),
+		},
+		{
+			name: "joined with a provider error",
+			err:  Safe(errors.Join(errors.New(providerErr), marked)),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := SafeMessage(tt.err)
+			if got != "connection refused" {
+				t.Errorf("SafeMessage() = %q, want %q", got, "connection refused")
+			}
+			if strings.Contains(got, "8019210420527506405") {
+				t.Errorf("SafeMessage() leaked wrapper text: %q", got)
+			}
+		})
+	}
+}
+
+func TestSafePreservesErrorsIs(t *testing.T) {
+	sentinel := errors.New("sentinel")
+
+	if !errors.Is(Safe(sentinel), sentinel) {
+		t.Error("Safe() broke errors.Is against the wrapped error")
+	}
+	if Safe(sentinel).Error() != "sentinel" {
+		t.Errorf("Safe().Error() = %q, want %q", Safe(sentinel).Error(), "sentinel")
+	}
+}
+
+func TestSafeMessageTruncates(t *testing.T) {
+	long := strings.Repeat("a", MaxConditionMessageLength+50)
+
+	got := SafeMessage(Safe(errors.New(long)))
+	want := strings.Repeat("a", MaxConditionMessageLength-len("...")) + "..."
+	if got != want {
+		t.Errorf("SafeMessage() length = %d, want %d", len(got), len(want))
+	}
+}
+
+// Truncation counts runes, so a multi-byte message is not cut mid-character.
+func TestSafeMessageTruncatesOnRunes(t *testing.T) {
+	long := strings.Repeat("é", MaxConditionMessageLength+10)
+
+	got := SafeMessage(Safe(errors.New(long)))
+	if runes := []rune(got); len(runes) != MaxConditionMessageLength {
+		t.Errorf("truncated to %d runes, want %d", len(runes), MaxConditionMessageLength)
+	}
+	if !strings.HasPrefix(got, "é") {
+		t.Errorf("truncation split a multi-byte rune: %q", got[:8])
+	}
+}
+
+// A limit too small to hold the marker still has to be respected.
+func TestTruncateLimitBelowMarker(t *testing.T) {
+	if got := truncate("abcdef", 2); got != "ab" {
+		t.Errorf("truncate() = %q, want %q", got, "ab")
+	}
+}