From 2c91c065366923bbe35e29ff7581536a1d87cdc3 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Wed, 10 Jun 2026 09:55:46 +0200 Subject: [PATCH 1/2] fix(controlplane): propagate FailedPrecondition errors instead of masking them When an error already converted by handleUseCaseErr is processed again (e.g. AttestationService.Store wrapping storeAttestation), the resulting gRPC FailedPrecondition status error was not recognized, so it was reported to Sentry and masked as a generic internal server error, hiding the actionable message (such as pushing an attestation to a released, immutable project version) from the client. Pass through gRPC FailedPrecondition status errors in the default branch, making the conversion idempotent: the error is no longer reported to Sentry and the original message reaches the client. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: 43473f24-f00c-4ba7-8fac-82209c736b02 --- app/controlplane/internal/service/service.go | 13 ++++ .../internal/service/service_test.go | 69 ++++++++++++++++++- 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/app/controlplane/internal/service/service.go b/app/controlplane/internal/service/service.go index 5f1ffd1ad..ff1367b23 100644 --- a/app/controlplane/internal/service/service.go +++ b/app/controlplane/internal/service/service.go @@ -422,6 +422,19 @@ func handleUseCaseErr(err error, l *log.Helper) error { case biz.IsErrReleasedVersionImmutable(err): return status.Error(codes.FailedPrecondition, err.Error()) default: + // Business precondition failures (e.g. adding attestations to a released project version) + // are client errors: propagate them instead of masking and reporting them to Sentry. + // They can reach this point as gRPC status errors when an error already converted by + // this function is processed again (e.g. Store wrapping storeAttestation). + // We extract the status via GRPCStatus() instead of status.FromError because the latter + // rewrites the message of wrapped errors with the "rpc error: ..." prefix. + var gs interface{ GRPCStatus() *status.Status } + if errors.As(err, &gs) { + if s := gs.GRPCStatus(); s.Code() == codes.FailedPrecondition { + return s.Err() + } + } + return servicelogger.LogAndMaskErr(err, l) } } diff --git a/app/controlplane/internal/service/service_test.go b/app/controlplane/internal/service/service_test.go index 9be990b20..593665b16 100644 --- a/app/controlplane/internal/service/service_test.go +++ b/app/controlplane/internal/service/service_test.go @@ -1,5 +1,5 @@ // -// Copyright 2023 The Chainloop Authors. +// Copyright 2023-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -17,13 +17,80 @@ package service import ( "context" + "errors" + "fmt" "testing" "github.com/chainloop-dev/chainloop/app/controlplane/internal/usercontext/entities" + "github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz" + kerrors "github.com/go-kratos/kratos/v2/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) +func TestHandleUseCaseErr(t *testing.T) { + t.Parallel() + + immutableErr := status.Error(codes.FailedPrecondition, `version "v1.83.2+next" is released and immutable: attestations cannot be added`) + + testCases := []struct { + name string + err error + wantCode codes.Code + wantMessage string + }{ + { + name: "failed precondition status error is propagated", + err: immutableErr, + wantCode: codes.FailedPrecondition, + wantMessage: `version "v1.83.2+next" is released and immutable: attestations cannot be added`, + }, + { + name: "wrapped failed precondition keeps code and original message", + err: fmt.Errorf("saving attestation digest: %w", immutableErr), + wantCode: codes.FailedPrecondition, + wantMessage: `version "v1.83.2+next" is released and immutable: attestations cannot be added`, + }, + { + name: "released version immutable biz error maps to failed precondition", + err: fmt.Errorf("saving attestation digest: %w", biz.NewErrReleasedVersionImmutable("v1.83.2+next")), + wantCode: codes.FailedPrecondition, + wantMessage: `saving attestation digest: version "v1.83.2+next" is released and immutable: attestations cannot be added`, + }, + { + name: "already converted error is propagated unchanged when processed again", + err: handleUseCaseErr(fmt.Errorf("saving attestation digest: %w", biz.NewErrReleasedVersionImmutable("v1.83.2+next")), nil), + wantCode: codes.FailedPrecondition, + wantMessage: `saving attestation digest: version "v1.83.2+next" is released and immutable: attestations cannot be added`, + }, + { + name: "validation error maps to bad request", + err: biz.NewErrValidationStr("invalid input"), + wantCode: codes.InvalidArgument, + wantMessage: "validation error: invalid input", + }, + { + name: "unknown error is masked as internal server error", + err: errors.New("sensitive details"), + wantCode: codes.Internal, + wantMessage: "server error", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got := handleUseCaseErr(tc.err, nil) + require.Error(t, got) + assert.Equal(t, tc.wantCode, status.Code(got)) + assert.Equal(t, tc.wantMessage, kerrors.FromError(got).GetMessage()) + }) + } +} + func TestRequireCurrentUser(t *testing.T) { t.Parallel() From 5e2c2afe247a1ec0a530b4ecabec2ae580618931 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Wed, 10 Jun 2026 12:02:53 +0200 Subject: [PATCH 2/2] fix(controlplane): make handleUseCaseErr idempotent for all client-error codes Extend the FailedPrecondition passthrough to every client-error gRPC code this function produces (Canceled, InvalidArgument, NotFound, PermissionDenied, Unimplemented, AlreadyExists, FailedPrecondition). Kratos errors also implement GRPCStatus(), so already-converted errors of any of these kinds are now propagated unchanged when processed again, instead of being masked and reported to Sentry. Server-side codes keep being masked. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: 43473f24-f00c-4ba7-8fac-82209c736b02 --- app/controlplane/internal/service/service.go | 21 ++++++++++++++----- .../internal/service/service_test.go | 18 ++++++++++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/app/controlplane/internal/service/service.go b/app/controlplane/internal/service/service.go index ff1367b23..40e613b5e 100644 --- a/app/controlplane/internal/service/service.go +++ b/app/controlplane/internal/service/service.go @@ -422,15 +422,14 @@ func handleUseCaseErr(err error, l *log.Helper) error { case biz.IsErrReleasedVersionImmutable(err): return status.Error(codes.FailedPrecondition, err.Error()) default: - // Business precondition failures (e.g. adding attestations to a released project version) - // are client errors: propagate them instead of masking and reporting them to Sentry. - // They can reach this point as gRPC status errors when an error already converted by - // this function is processed again (e.g. Store wrapping storeAttestation). + // Client errors already converted by this function can be processed again + // (e.g. AttestationService.Store wraps storeAttestation, which converts internally). + // Propagate them instead of masking and reporting them to Sentry. // We extract the status via GRPCStatus() instead of status.FromError because the latter // rewrites the message of wrapped errors with the "rpc error: ..." prefix. var gs interface{ GRPCStatus() *status.Status } if errors.As(err, &gs) { - if s := gs.GRPCStatus(); s.Code() == codes.FailedPrecondition { + if s := gs.GRPCStatus(); isClientErrorCode(s.Code()) { return s.Err() } } @@ -438,3 +437,15 @@ func handleUseCaseErr(err error, l *log.Helper) error { return servicelogger.LogAndMaskErr(err, l) } } + +// isClientErrorCode returns true for the gRPC client-error codes that handleUseCaseErr +// produces, making its conversion idempotent: server-side codes keep being masked +func isClientErrorCode(c codes.Code) bool { + switch c { + case codes.Canceled, codes.InvalidArgument, codes.NotFound, codes.PermissionDenied, + codes.Unimplemented, codes.AlreadyExists, codes.FailedPrecondition: + return true + default: + return false + } +} diff --git a/app/controlplane/internal/service/service_test.go b/app/controlplane/internal/service/service_test.go index 593665b16..939405671 100644 --- a/app/controlplane/internal/service/service_test.go +++ b/app/controlplane/internal/service/service_test.go @@ -65,6 +65,24 @@ func TestHandleUseCaseErr(t *testing.T) { wantCode: codes.FailedPrecondition, wantMessage: `saving attestation digest: version "v1.83.2+next" is released and immutable: attestations cannot be added`, }, + { + name: "already converted not found error is propagated unchanged when processed again", + err: handleUseCaseErr(biz.NewErrNotFound("workflow"), nil), + wantCode: codes.NotFound, + wantMessage: "workflow not found", + }, + { + name: "already converted already exists error is propagated unchanged when processed again", + err: handleUseCaseErr(biz.NewErrAlreadyExists(errors.New("name taken")), nil), + wantCode: codes.AlreadyExists, + wantMessage: "duplicated: name taken", + }, + { + name: "server-side status error is still masked", + err: status.Error(codes.Unavailable, "connection to database lost"), + wantCode: codes.Internal, + wantMessage: "server error", + }, { name: "validation error maps to bad request", err: biz.NewErrValidationStr("invalid input"),