From 03c39fd922d8ea86e9198ae43c527d49be60cee5 Mon Sep 17 00:00:00 2001 From: anirudhwarrier <12178754+anirudhwarrier@users.noreply.github.com> Date: Thu, 15 Oct 2026 07:14:34 -0400 Subject: [PATCH] fix(secrets): exit non-zero when the vault rejects a secret Co-Authored-By: Claude Opus 5.5 --- .changeset/secrets-exit-code.md | 5 ++ cmd/secrets/common/browser_flow_test.go | 2 +- cmd/secrets/common/handler.go | 25 ++++++- cmd/secrets/common/parse_response_test.go | 69 +++++++++++++++++-- .../common/vault_response_verify_test.go | 6 +- 5 files changed, 99 insertions(+), 8 deletions(-) create mode 100644 .changeset/secrets-exit-code.md diff --git a/.changeset/secrets-exit-code.md b/.changeset/secrets-exit-code.md new file mode 100644 index 00000000..0f75d8c6 --- /dev/null +++ b/.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. diff --git a/cmd/secrets/common/browser_flow_test.go b/cmd/secrets/common/browser_flow_test.go index be1b2439..a22fe4fe 100644 --- a/cmd/secrets/common/browser_flow_test.go +++ b/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") } diff --git a/cmd/secrets/common/handler.go b/cmd/secrets/common/handler.go index 3ae32b08..0da77bbb 100644 --- a/cmd/secrets/common/handler.go +++ b/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) { diff --git a/cmd/secrets/common/parse_response_test.go b/cmd/secrets/common/parse_response_test.go index 6ed1b4cb..edf73bbe 100644 --- a/cmd/secrets/common/parse_response_test.go +++ b/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)))) +} diff --git a/cmd/secrets/common/vault_response_verify_test.go b/cmd/secrets/common/vault_response_verify_test.go index bd423963..bd4a6b00 100644 --- a/cmd/secrets/common/vault_response_verify_test.go +++ b/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) {