fix(secrets): exit non-zero when the vault rejects a secret

.changeset/secrets-exit-code.md

@@ -0,0 +1,5 @@
+---
+"cre-cli": patch
+---
+
+`cre secrets create`, `update`, `delete` and `list` now exit non-zero when the Vault rejects a request (for example "key does not exist" on update). Previously the failure was only printed and the command exited 0, so scripts and CI treated it as success.

cmd/secrets/common/browser_flow_test.go

@@ -81,7 +81,7 @@ func TestPostVaultGatewayWithBearer_CreateParsesResponse(t *testing.T) {
 	var out strings.Builder
 	_, _ = io.Copy(&out, r)
 
-	require.NoError(t, err)
+	require.EqualError(t, err, "1 of 3 secret(s) failed to create")
 	assert.Contains(t, out.String(), "Secret created")
 }
 

cmd/secrets/common/handler.go

@@ -687,6 +687,7 @@ func (h *Handler) Execute(
 // ParseVaultGatewayResponse parses the JSON-RPC response, optionally verifies OCR signatures
 // and the JSON-RPC response id, decodes the SignedOCRResponse payload into the appropriate proto
 // type, and logs one line per secret with id/owner/namespace/success/error.
+// It returns an error when the vault rejects any secret, so the command exits non-zero.
 func (h *Handler) ParseVaultGatewayResponse(method, requestID string, respBody []byte) error {
 	// Unmarshal JSON-RPC envelope with SignedOCRResponse result
 	var rpcResp jsonrpc2.Response[vaulttypes.SignedOCRResponse]
@@ -717,6 +718,7 @@ func (h *Handler) ParseVaultGatewayResponse(method, requestID string, respBody [
 			return fmt.Errorf("failed to decode create payload: %w", err)
 		}
 
+		failed := 0
 		for _, r := range p.GetResponses() {
 			id := r.GetId()
 			// Safeguard for nil id
@@ -727,15 +729,20 @@ func (h *Handler) ParseVaultGatewayResponse(method, requestID string, respBody [
 			if r.GetSuccess() {
 				ui.Success(fmt.Sprintf("Secret created: secret_id=%s, owner=%s, namespace=%s", key, owner, ns))
 			} else {
+				failed++
 				ui.Error(fmt.Sprintf("Secret create failed: secret_id=%s owner=%s namespace=%s error=%s",
 					key, owner, ns, r.GetError()))
 			}
 		}
+		if err := secretFailuresErr("create", failed, len(p.GetResponses())); err != nil {
+			return err
+		}
 	case vaulttypes.MethodSecretsUpdate:
 		var p vault.UpdateSecretsResponse
 		if err := unmarshalVaultResponsePayload(rpcResp.Result.Payload, &p); err != nil {
 			return fmt.Errorf("failed to decode update payload: %w", err)
 		}
+		failed := 0
 		for _, r := range p.GetResponses() {
 			id := r.GetId()
 			key, owner, ns := "", "", ""
@@ -745,15 +752,20 @@ func (h *Handler) ParseVaultGatewayResponse(method, requestID string, respBody [
 			if r.GetSuccess() {
 				ui.Success(fmt.Sprintf("Secret updated: secret_id=%s, owner=%s, namespace=%s", key, owner, ns))
 			} else {
+				failed++
 				ui.Error(fmt.Sprintf("Secret update failed: secret_id=%s owner=%s namespace=%s error=%s",
 					key, owner, ns, r.GetError()))
 			}
 		}
+		if err := secretFailuresErr("update", failed, len(p.GetResponses())); err != nil {
+			return err
+		}
 	case vaulttypes.MethodSecretsDelete:
 		var p vault.DeleteSecretsResponse
 		if err := unmarshalVaultResponsePayload(rpcResp.Result.Payload, &p); err != nil {
 			return fmt.Errorf("failed to decode delete payload: %w", err)
 		}
+		failed := 0
 		for _, r := range p.GetResponses() {
 			id := r.GetId()
 			key, owner, ns := "", "", ""
@@ -763,10 +775,14 @@ func (h *Handler) ParseVaultGatewayResponse(method, requestID string, respBody [
 			if r.GetSuccess() {
 				ui.Success(fmt.Sprintf("Secret deleted: secret_id=%s, owner=%s, namespace=%s", key, owner, ns))
 			} else {
+				failed++
 				ui.Error(fmt.Sprintf("Secret delete failed: secret_id=%s owner=%s namespace=%s error=%s",
 					key, owner, ns, r.GetError()))
 			}
 		}
+		if err := secretFailuresErr("delete", failed, len(p.GetResponses())); err != nil {
+			return err
+		}
 	case vaulttypes.MethodSecretsList:
 		var p vault.ListSecretIdentifiersResponse
 		if err := unmarshalVaultResponsePayload(rpcResp.Result.Payload, &p); err != nil {
@@ -775,7 +791,7 @@ func (h *Handler) ParseVaultGatewayResponse(method, requestID string, respBody [
 
 		if !p.GetSuccess() {
 			ui.Error(fmt.Sprintf("Secret list failed: error=%s", p.GetError()))
-			break
+			return fmt.Errorf("secret list failed: %s", p.GetError())
 		}
 
 		ids := p.GetIdentifiers()
@@ -800,6 +816,13 @@ func (h *Handler) ParseVaultGatewayResponse(method, requestID string, respBody [
 	return nil
 }
 
+func secretFailuresErr(op string, failed, total int) error {
+	if failed == 0 {
+		return nil
+	}
+	return fmt.Errorf("%d of %d secret(s) failed to %s", failed, total, op)
+}
+
 // EnsureOwnerLinkedOrFail TODO this reuses the same logic as in auto_link.go which is tied to deploy; consider refactoring to avoid duplication
 func (h *Handler) EnsureOwnerLinkedOrFail(ctx context.Context) error {
 	if !common.IsHexAddress(h.OwnerAddress) {

cmd/secrets/common/parse_response_test.go

@@ -10,7 +10,9 @@ import (
 	"testing"
 
 	"github.com/rs/zerolog"
+	"github.com/stretchr/testify/require"
 	"google.golang.org/protobuf/encoding/protojson"
+	"google.golang.org/protobuf/proto"
 
 	"github.com/smartcontractkit/chainlink-common/pkg/capabilities/actions/vault"
 	"github.com/smartcontractkit/chainlink/v2/core/capabilities/vault/vaulttypes"
@@ -134,8 +136,9 @@ func TestParseVaultGatewayResponse_Create_LogsPerItem(t *testing.T) {
 	h := newTestHandler(&buf)
 
 	body := encodeRPCBodyFromPayload(buildCreatePayloadProto(t))
-	if err := h.ParseVaultGatewayResponse(vaulttypes.MethodSecretsCreate, "", body); err != nil {
-		t.Fatalf("unexpected error: %v", err)
+	err := h.ParseVaultGatewayResponse(vaulttypes.MethodSecretsCreate, "", body)
+	if err == nil || err.Error() != "1 of 3 secret(s) failed to create" {
+		t.Fatalf("expected the rejected secret to fail the command, got: %v", err)
 	}
 
 	wOut.Close()
@@ -403,8 +406,9 @@ func TestParseVaultGatewayResponse_List_Failure(t *testing.T) {
 	h := newTestHandler(&buf)
 
 	body := encodeRPCBodyFromPayload(buildListPayloadProtoFailure(t, "boom"))
-	if err := h.ParseVaultGatewayResponse(vaulttypes.MethodSecretsList, "", body); err != nil {
-		t.Fatalf("unexpected error: %v", err)
+	err := h.ParseVaultGatewayResponse(vaulttypes.MethodSecretsList, "", body)
+	if err == nil || !strings.Contains(err.Error(), "secret list failed: boom") {
+		t.Fatalf("expected list failure error, got: %v", err)
 	}
 
 	wErr.Close()
@@ -435,3 +439,60 @@ func TestParseVaultGatewayResponse_BadPayloadForList(t *testing.T) {
 		t.Fatalf("expected decode error for list payload, got: %v", err)
 	}
 }
+
+// Scripts rely on the exit code, so a vault rejection must surface as an error for every mutating method.
+func TestParseVaultGatewayResponse_RejectedSecretsReturnError(t *testing.T) {
+	rejected := func(t *testing.T, msg proto.Message) []byte {
+		t.Helper()
+		b, err := protojson.Marshal(msg)
+		require.NoError(t, err)
+		return encodeRPCBodyFromPayload(b)
+	}
+	id := &vault.SecretIdentifier{Key: "E2E_API_TOKEN", Owner: "0xabc", Namespace: "main"}
+
+	tests := []struct {
+		name    string
+		method  string
+		body    []byte
+		wantErr string
+	}{
+		{
+			name:   "update of missing key",
+			method: vaulttypes.MethodSecretsUpdate,
+			body: rejected(t, &vault.UpdateSecretsResponse{Responses: []*vault.UpdateSecretResponse{
+				{Id: id, Success: false, Error: "could not write update to key value store: key does not exist"},
+			}}),
+			wantErr: "1 of 1 secret(s) failed to update",
+		},
+		{
+			name:   "delete of missing key",
+			method: vaulttypes.MethodSecretsDelete,
+			body: rejected(t, &vault.DeleteSecretsResponse{Responses: []*vault.DeleteSecretResponse{
+				{Id: id, Success: false, Error: "key does not exist"},
+			}}),
+			wantErr: "1 of 1 secret(s) failed to delete",
+		},
+		{
+			name:   "create of existing key",
+			method: vaulttypes.MethodSecretsCreate,
+			body: rejected(t, &vault.CreateSecretsResponse{Responses: []*vault.CreateSecretResponse{
+				{Id: id, Success: false, Error: "key already exists"},
+			}}),
+			wantErr: "1 of 1 secret(s) failed to create",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var buf bytes.Buffer
+			h := newTestHandler(&buf)
+			err := h.ParseVaultGatewayResponse(tt.method, "", tt.body)
+			require.EqualError(t, err, tt.wantErr)
+		})
+	}
+}
+
+func TestParseVaultGatewayResponse_AllSucceededReturnsNil(t *testing.T) {
+	var buf bytes.Buffer
+	h := newTestHandler(&buf)
+	require.NoError(t, h.ParseVaultGatewayResponse(vaulttypes.MethodSecretsDelete, "", encodeRPCBodyFromPayload(buildDeletePayloadProto(t))))
+}

cmd/secrets/common/vault_response_verify_test.go

@@ -137,8 +137,9 @@ func TestVerifyVaultGatewayResponse_EmptySignaturesPassThrough(t *testing.T) {
 	body, err := json.Marshal(resp)
 	require.NoError(t, err)
 
+	// Verification passes; the error comes from the fixture's rejected secret, not from signatures.
 	err = h.ParseVaultGatewayResponse(vaulttypes.MethodSecretsCreate, requestID, body)
-	require.NoError(t, err)
+	require.EqualError(t, err, "1 of 3 secret(s) failed to create")
 }
 
 func TestVerifyVaultGatewayResponse_ValidSignatures(t *testing.T) {
@@ -160,8 +161,9 @@ func TestVerifyVaultGatewayResponse_ValidSignatures(t *testing.T) {
 		"c7517c188d297093a6f602046fad7feafe19454ee9dc269b19c8e6c01268037d1f7b423eeecbc495dd2d9a65e106bc3eab849ddfd74a10cbd4ad50c7d953bd4b01",
 	)
 
+	// The signed payload carries one rejected secret; the signatures still verify.
 	err := h.ParseVaultGatewayResponse(vaulttypes.MethodSecretsCreate, requestID, body)
-	require.NoError(t, err)
+	require.EqualError(t, err, "1 of 1 secret(s) failed to create")
 }
 
 func TestVerifyVaultGatewayResponse_InvalidSignatures(t *testing.T) {