Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions app/controlplane/internal/service/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ import (
"google.golang.org/grpc/status"
)

// maskedClientMsg is the generic message servicelogger.LogAndMaskErr returns to
// the client, hiding the real (potentially sensitive) server-side error.
const maskedClientMsg = "server error"

func TestHandleUseCaseErr(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -81,7 +85,7 @@ func TestHandleUseCaseErr(t *testing.T) {
name: "server-side status error is still masked",
err: status.Error(codes.Unavailable, "connection to database lost"),
wantCode: codes.Internal,
wantMessage: "server error",
wantMessage: maskedClientMsg,
},
{
name: "validation error maps to bad request",
Expand All @@ -93,7 +97,28 @@ func TestHandleUseCaseErr(t *testing.T) {
name: "unknown error is masked as internal server error",
err: errors.New("sensitive details"),
wantCode: codes.Internal,
wantMessage: "server error",
wantMessage: maskedClientMsg,
},
{
// PFM-6775: a transient org-lookup failure now surfaces the real DB
// cause up the stack (wrapped with %w by the data and biz layers).
// It must be masked as a generic internal error so the SQL detail
// never reaches the client, while the real error remains available
// for logging/Sentry.
name: "transient org-lookup failure is masked, SQL detail not leaked to client",
err: fmt.Errorf("failed to find membership: %w",
fmt.Errorf("querying organization %q: %w", "chainloop",
errors.New("pq: canceling statement due to statement timeout (SQLSTATE 57014)"))),
wantCode: codes.Internal,
wantMessage: maskedClientMsg,
},
{
// PFM-6775: a genuinely missing org still maps to NotFound (never
// masked), so ContextService.Current can degrade gracefully.
name: "genuinely missing org maps to not found",
err: fmt.Errorf("failed to find membership: %w", biz.NewErrNotFound("organization chainloop")),
wantCode: codes.NotFound,
wantMessage: "failed to find membership: organization chainloop not found",
},
}

Expand Down
41 changes: 41 additions & 0 deletions app/controlplane/pkg/biz/membership_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package biz_test

import (
"context"
"errors"
"fmt"
"testing"

Expand Down Expand Up @@ -651,6 +652,46 @@ func TestMembershipUseCase(t *testing.T) {
suite.Run(t, new(membershipFilteringPaginationTestSuite))
}

// FindByOrgNameAndUser must not mask transient (non-NotFound) failures of the
// org lookup as a fabricated "organization not found". A genuinely missing org
// still has to surface as biz.NotFound so that ContextService.Current can
// degrade gracefully instead of returning a captured gRPC 500 (see PFM-6775).
func (s *membershipIntegrationTestSuite) TestFindByOrgNameAndUserPreservesTransientError() {
ctx := context.Background()

org, err := s.Organization.CreateWithRandomName(ctx)
s.NoError(err)
user, err := s.User.UpsertByEmail(ctx, "transient@test.com", nil)
s.NoError(err)

userUUID, err := uuid.Parse(user.ID)
s.NoError(err)

s.Run("transient failure surfaces the real error, not a fake not-found", func() {
// A cancelled context makes the org query fail with a non-NotFound
// error against an org that actually exists. The underlying cause must
// be preserved (wrapped with %w) rather than masked. This asserts the
// data-layer contract ("non-NotFound is preserved"); how each transient
// class then maps to a gRPC status/Sentry is covered by the
// service-layer TestHandleUseCaseErr cases.
canceledCtx, cancel := context.WithCancel(ctx)
cancel()

_, err := s.Repos.Membership.FindByOrgNameAndUser(canceledCtx, org.Name, userUUID)
s.Error(err)
// The transient cause must remain observable...
s.True(errors.Is(err, context.Canceled), "expected the real error to be preserved, got: %v", err)
// ...and it must NOT be masked as a biz.NotFound.
s.False(biz.IsNotFound(err), "a transient failure must not be reported as not-found")
})

s.Run("genuinely missing org still returns biz.NotFound", func() {
_, err := s.Repos.Membership.FindByOrgNameAndUser(ctx, "non-existent-org-"+uuid.NewString(), userUUID)
s.Error(err)
s.True(biz.IsNotFound(err), "a missing org must map to biz.NotFound")
})
}

// Utility struct to hold the test suite
type membershipIntegrationTestSuite struct {
testhelpers.UseCasesEachTestSuite
Expand Down
6 changes: 5 additions & 1 deletion app/controlplane/pkg/data/membership.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,11 @@ func (r *MembershipRepo) FindByOrgNameAndUser(ctx context.Context, orgName strin
if ent.IsNotFound(err) {
return nil, biz.NewErrNotFound(fmt.Sprintf("organization %s not found", orgName))
}
return nil, fmt.Errorf("organization %s not found", orgName)
// Preserve the real error (e.g. context cancellation, statement/pool
// timeout) instead of masking a transient failure as a fake "not found".
// A fabricated not-found here would surface as a captured gRPC 500 in
// ContextService.Current rather than degrading gracefully (see PFM-6775).
return nil, fmt.Errorf("querying organization %q: %w", orgName, err)
}

m, err := r.data.DB.Membership.Query().Where(
Expand Down
Loading