From 3da735c86a58a1e83ad25378d9922fe52a8c6e6c Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Sat, 31 Oct 2026 23:55:16 +0100 Subject: [PATCH 1/3] feat: record error stacks at external client boundaries Use pkg/errorsx where errors from external clients enter Chainloop, so Sentry groups failures by where they start instead of by the shared capture site. - grpcconn: add stack-recording unary and stream client interceptors, enabled by default and exported for reuse. io.EOF stays unwrapped. - Plugin SDK HTTP client: a transport that records a stack on each round-trip error. - Blob backends (OCI, S3, S3 access point, Azure Blob), secret backends (Vault, AWS, GCP, Azure Key Vault), NATS, the OIDC provider, the EJBCA and file CAs, the policy provider and remote policy loaders. - Distinct error types for non-2xx responses in EJBCA, the policy provider, the remote policy loader and Dependency-Track. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: b3cc191e-bd8c-4933-beb5-9a5ae1f2331c --- .../internal/oidcauthenticator/auth.go | 15 +- app/controlplane/internal/service/auth.go | 3 +- app/controlplane/pkg/auditor/nats.go | 11 +- app/controlplane/pkg/biz/signing.go | 18 +- app/controlplane/pkg/ca/ejbca/ejbca.go | 57 ++++-- app/controlplane/pkg/ca/ejbca/ejbca_test.go | 30 ++++ app/controlplane/pkg/ca/fileca/fileca.go | 24 +-- app/controlplane/pkg/ca/fileca/fileca_test.go | 51 ++++++ .../pkg/policies/policyprovider.go | 52 ++++-- .../pkg/policies/policyprovider_http_test.go | 22 +++ .../core/dependency-track/v1/client/sbom.go | 35 +++- .../dependency-track/v1/client/sbom_test.go | 52 ++++++ .../core/discord-webhook/v1/discord.go | 7 +- app/controlplane/plugins/core/guac/v1/guac.go | 13 +- .../core/slack-webhook/v1/slack_webhook.go | 3 +- .../plugins/core/smtp/v1/extension.go | 5 +- .../plugins/core/webhook/v1/webhook.go | 3 +- app/controlplane/plugins/sdk/v1/httpclient.go | 21 ++- .../plugins/sdk/v1/httpclient_test.go | 6 +- pkg/blobmanager/azureblob/backend.go | 28 +-- pkg/blobmanager/oci/backend.go | 59 +++---- pkg/blobmanager/s3/backend.go | 27 +-- pkg/blobmanager/s3accesspoint/backend.go | 28 +-- pkg/cache/natskv.go | 9 +- pkg/credentials/aws/secretmanager.go | 13 +- pkg/credentials/azurekv/keyvault.go | 27 +-- pkg/credentials/gcp/secretmanager.go | 18 +- pkg/credentials/vault/keyval.go | 21 +-- pkg/grpcconn/grpcconn.go | 16 +- pkg/grpcconn/stack.go | 73 ++++++++ pkg/grpcconn/stack_test.go | 163 ++++++++++++++++++ pkg/natsconn/natsconn.go | 4 +- pkg/policies/fetch_remote_test.go | 73 ++++++++ pkg/policies/loader.go | 19 +- 34 files changed, 800 insertions(+), 206 deletions(-) create mode 100644 app/controlplane/pkg/ca/ejbca/ejbca_test.go create mode 100644 app/controlplane/pkg/ca/fileca/fileca_test.go create mode 100644 pkg/grpcconn/stack.go create mode 100644 pkg/grpcconn/stack_test.go create mode 100644 pkg/policies/fetch_remote_test.go diff --git a/app/controlplane/internal/oidcauthenticator/auth.go b/app/controlplane/internal/oidcauthenticator/auth.go index f89e87e9f..0e279b0ca 100644 --- a/app/controlplane/internal/oidcauthenticator/auth.go +++ b/app/controlplane/internal/oidcauthenticator/auth.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,8 +17,8 @@ package authenticator import ( "context" - "errors" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/coreos/go-oidc/v3/oidc" "golang.org/x/oauth2" ) @@ -34,7 +34,7 @@ type OIDC struct { func NewOIDC(discoveryDomain, clientID, clientSecret, redirectURL string) (*OIDC, error) { provider, err := oidc.NewProvider(context.Background(), discoveryDomain) if err != nil { - return nil, err + return nil, errorsx.WithStack(err) } conf := oauth2.Config{ @@ -56,11 +56,16 @@ func NewOIDC(discoveryDomain, clientID, clientSecret, redirectURL string) (*OIDC func (a *OIDC) VerifyIDToken(ctx context.Context, token *oauth2.Token) (*oidc.IDToken, error) { rawIDToken, ok := token.Extra("id_token").(string) if !ok { - return nil, errors.New("no id_token field in oauth2 token") + return nil, errorsx.New("no id_token field in oauth2 token") } // default settings oidcConfig := &oidc.Config{ClientID: a.ClientID} - return a.Verifier(oidcConfig).Verify(ctx, rawIDToken) + idToken, err := a.Verifier(oidcConfig).Verify(ctx, rawIDToken) + if err != nil { + return nil, errorsx.WithStack(err) + } + + return idToken, nil } diff --git a/app/controlplane/internal/service/auth.go b/app/controlplane/internal/service/auth.go index 810c9b274..fa07f8777 100644 --- a/app/controlplane/internal/service/auth.go +++ b/app/controlplane/internal/service/auth.go @@ -37,6 +37,7 @@ import ( "github.com/chainloop-dev/chainloop/app/controlplane/pkg/jwt" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/jwt/user" "github.com/chainloop-dev/chainloop/internal/oauth" + "github.com/chainloop-dev/chainloop/pkg/errorsx" sl "github.com/chainloop-dev/chainloop/pkg/servicelogger" kerrors "github.com/go-kratos/kratos/v2/errors" "github.com/go-kratos/kratos/v2/log" @@ -700,7 +701,7 @@ func extractUserInfoFromToken(ctx context.Context, svc *AuthService, r *http.Req // Exchange the code for a token oauth2Token, err := svc.authenticator.Exchange(ctx, code) if err != nil { - return nil, newOauthResp(http.StatusUnauthorized, err, false) + return nil, newOauthResp(http.StatusUnauthorized, errorsx.WithStack(err), false) } // It's a valid Oauth2 token diff --git a/app/controlplane/pkg/auditor/nats.go b/app/controlplane/pkg/auditor/nats.go index a11349c00..89deacade 100644 --- a/app/controlplane/pkg/auditor/nats.go +++ b/app/controlplane/pkg/auditor/nats.go @@ -22,6 +22,7 @@ import ( "strings" "time" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/chainloop-dev/chainloop/pkg/natsconn" "github.com/go-kratos/kratos/v2/log" "github.com/nats-io/nats.go/jetstream" @@ -85,7 +86,7 @@ func newPublisher(rc *natsconn.ReloadableConnection, logger log.Logger) *AuditLo func (p *AuditLogPublisher) initJetStream() error { js, err := jetstream.New(p.rc.Conn) if err != nil { - return fmt.Errorf("creating jetstream context: %w", err) + return errorsx.Errorf("creating jetstream context: %w", err) } ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) @@ -96,7 +97,7 @@ func (p *AuditLogPublisher) initJetStream() error { Subjects: []string{subjectName}, Replicas: p.rc.Replicas, }); err != nil { - return fmt.Errorf("creating stream: %w", err) + return errorsx.Errorf("creating stream: %w", err) } p.logger.Infow("msg", "stream created or updated", "name", streamName, "subject", subjectName, "replicas", p.rc.Replicas) @@ -124,5 +125,9 @@ func (p *AuditLogPublisher) Publish(data *EventPayload) error { } specificSubject := fmt.Sprintf("%s.%s.%s", baseSubjectName, strings.ToLower(string(data.Data.TargetType)), strings.ToLower(data.Data.ActionType)) - return p.rc.Publish(specificSubject, jsonPayload) + if err := p.rc.Publish(specificSubject, jsonPayload); err != nil { + return errorsx.Errorf("publishing audit event to %s: %w", specificSubject, err) + } + + return nil } diff --git a/app/controlplane/pkg/biz/signing.go b/app/controlplane/pkg/biz/signing.go index b1d13b48f..0d3d53c4c 100644 --- a/app/controlplane/pkg/biz/signing.go +++ b/app/controlplane/pkg/biz/signing.go @@ -23,13 +23,13 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/hex" - "errors" "fmt" "net/url" "os" conf "github.com/chainloop-dev/chainloop/app/controlplane/internal/conf/controlplane/config/v1" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/ca" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/chainloop-dev/chainloop/pkg/otelx" "github.com/chainloop-dev/chainloop/pkg/servicelogger" "github.com/go-kratos/kratos/v2/log" @@ -188,24 +188,24 @@ func (s *SigningUseCase) CreateSigningCert(ctx context.Context, orgID string, cs var publicKey crypto.PublicKey if len(csrRaw) == 0 { - return nil, errors.New("csr cannot be empty") + return nil, errorsx.New("csr cannot be empty") } // Parse CSR csr, err := cryptoutils.ParseCSR(csrRaw) if err != nil { - return nil, fmt.Errorf("parsing csr: %w", err) + return nil, errorsx.Errorf("parsing csr: %w", err) } // Parse public key and check for weak key parameters publicKey = csr.PublicKey if err := cryptoutils.ValidatePubKey(publicKey); err != nil { - return nil, fmt.Errorf("invalid public key: %w", err) + return nil, errorsx.Errorf("invalid public key: %w", err) } // Check the CSR signature is valid if err := csr.CheckSignature(); err != nil { - return nil, fmt.Errorf("invalid signature: %w", err) + return nil, errorsx.Errorf("invalid signature: %w", err) } // Create certificate from CA provider (no Signed Certificate Timestamps for now) @@ -221,13 +221,13 @@ func (s *SigningUseCase) CreateSigningCert(ctx context.Context, orgID string, cs // Generated certificate finalPEM, err := csc.CertPEM() if err != nil { - return nil, fmt.Errorf("marshaling certificate to PEM: %w", err) + return nil, errorsx.Errorf("marshaling certificate to PEM: %w", err) } // Certificate chain finalChainPEM, err := csc.ChainPEM() if err != nil { - return nil, fmt.Errorf("marshaling chain to PEM: %w", err) + return nil, errorsx.Errorf("marshaling chain to PEM: %w", err) } return append([]string{finalPEM}, finalChainPEM...), nil @@ -254,7 +254,7 @@ func (s *SigningUseCase) GetTrustedRoot(ctx context.Context) (*TrustedRoot, erro for _, cert := range chain { pemCert, err := cryptoutils.MarshalCertificateToPEM(cert) if err != nil { - return nil, fmt.Errorf("marshaling certificate to PEM: %w", err) + return nil, errorsx.Errorf("marshaling certificate to PEM: %w", err) } trustedRoot.Keys[keyID] = append(trustedRoot.Keys[keyID], string(pemCert)) } @@ -270,7 +270,7 @@ func (s *SigningUseCase) GetTrustedRoot(ctx context.Context) (*TrustedRoot, erro for _, cert := range authority.CertChain { pemCert, err := cryptoutils.MarshalCertificateToPEM(cert) if err != nil { - return nil, fmt.Errorf("marshaling certificate to PEM: %w", err) + return nil, errorsx.Errorf("marshaling certificate to PEM: %w", err) } trustedRoot.TimestampAuthorities[authorityKeyID] = append(trustedRoot.TimestampAuthorities[authorityKeyID], string(pemCert)) } diff --git a/app/controlplane/pkg/ca/ejbca/ejbca.go b/app/controlplane/pkg/ca/ejbca/ejbca.go index 2d8e1aaf8..3fd55d4be 100644 --- a/app/controlplane/pkg/ca/ejbca/ejbca.go +++ b/app/controlplane/pkg/ca/ejbca/ejbca.go @@ -1,5 +1,5 @@ // -// Copyright 2024 The Chainloop Authors. +// Copyright 2024-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. @@ -28,6 +28,7 @@ import ( "net/http" "os" + "github.com/chainloop-dev/chainloop/pkg/errorsx" fulcioca "github.com/sigstore/fulcio/pkg/ca" "github.com/sigstore/fulcio/pkg/identity" "github.com/sigstore/sigstore/pkg/cryptoutils" @@ -35,6 +36,20 @@ import ( const CAName = "ejbCA" +// EnrollStatusError is returned when EJBCA answers a certificate enrollment +// with an unexpected HTTP status. It is a separate type so that error trackers +// do not group it with transport or decoding failures. +type EnrollStatusError struct { + // Status is the HTTP status line, for example "403 Forbidden". + Status string + // StatusCode is the HTTP status code. + StatusCode int +} + +func (e *EnrollStatusError) Error() string { + return fmt.Sprintf("wrong status creating certificate: %v", e.Status) +} + type EJBCA struct { // Connection settings // EJBCA installation. ie https://localhost/ejbca/ @@ -55,7 +70,7 @@ type EJBCA struct { func New(serverURL, keyPath, certPath, rootCAPath, certProfileName, endEntityProfileName, caName string) (*EJBCA, error) { if serverURL == "" || keyPath == "" || certPath == "" || certProfileName == "" || endEntityProfileName == "" || caName == "" { - return nil, fmt.Errorf("ejbca: invalid arguments") + return nil, errorsx.New("ejbca: invalid arguments") } return &EJBCA{ @@ -102,12 +117,12 @@ func (e EJBCA) CreateCertificateFromCSR(ctx context.Context, principal identity. } ejbcaReqBytes, err := json.Marshal(ejbcaReq) if err != nil { - return nil, fmt.Errorf("failed to marshal EJBCA Request: %w", err) + return nil, errorsx.Errorf("failed to marshal EJBCA Request: %w", err) } cert, err := tls.LoadX509KeyPair(e.certPath, e.keyPath) if err != nil { - return nil, fmt.Errorf("error loading X509 key pair: %w", err) + return nil, errorsx.Errorf("error loading X509 key pair: %w", err) } tlsConfig := &tls.Config{ @@ -118,7 +133,7 @@ func (e EJBCA) CreateCertificateFromCSR(ctx context.Context, principal identity. if e.rootCAPath != "" { caCert, err := os.ReadFile(e.rootCAPath) if err != nil { - return nil, fmt.Errorf("error reading root CA: %w", err) + return nil, errorsx.Errorf("error reading root CA: %w", err) } caCertPool := x509.NewCertPool() caCertPool.AppendCertsFromPEM(caCert) @@ -131,7 +146,7 @@ func (e EJBCA) CreateCertificateFromCSR(ctx context.Context, principal identity. req, err := http.NewRequest("POST", fmt.Sprintf("%s/ejbca-rest-api/v1/certificate/pkcs10enroll", e.serverURL), bytes.NewReader(ejbcaReqBytes)) if err != nil { - return nil, fmt.Errorf("unable to create request: %w", err) + return nil, errorsx.Errorf("unable to create request: %w", err) } req.Header.Set("Content-Type", "application/json") @@ -139,26 +154,27 @@ func (e EJBCA) CreateCertificateFromCSR(ctx context.Context, principal identity. resp, err := client.Do(req) if err != nil { - return nil, fmt.Errorf("unable to send request: %w", err) + return nil, errorsx.Errorf("unable to send request: %w", err) } + defer resp.Body.Close() if resp.StatusCode != http.StatusCreated { - return nil, fmt.Errorf("wrong status creating certificate: %v", resp.Status) + return nil, errorsx.WithStack(&EnrollStatusError{Status: resp.Status, StatusCode: resp.StatusCode}) } var rsp response body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("unable to read response body: %w", err) + return nil, errorsx.Errorf("unable to read response body: %w", err) } if err := json.Unmarshal(body, &rsp); err != nil { - return nil, fmt.Errorf("unable to parse response body: %w", err) + return nil, errorsx.Errorf("unable to parse response body: %w", err) } // Decode certificate derCert, err := base64.StdEncoding.DecodeString(rsp.Certificate) if err != nil { - return nil, fmt.Errorf("unable to decode response body: %w", err) + return nil, errorsx.Errorf("unable to decode response body: %w", err) } // Decode chain @@ -166,16 +182,21 @@ func (e EJBCA) CreateCertificateFromCSR(ctx context.Context, principal identity. for _, c := range rsp.CertificateChain { decodedCert, err := base64.StdEncoding.DecodeString(c) if err != nil { - return nil, fmt.Errorf("unable to decode response body: %w", err) + return nil, errorsx.Errorf("unable to decode response body: %w", err) } cert, err := x509.ParseCertificate(decodedCert) if err != nil { - return nil, fmt.Errorf("unable to parse cert chain: %w", err) + return nil, errorsx.Errorf("unable to parse cert chain: %w", err) } chain = append(chain, cert) } - return fulcioca.CreateCSCFromDER(derCert, chain) + csc, err := fulcioca.CreateCSCFromDER(derCert, chain) + if err != nil { + return nil, errorsx.Errorf("unable to create code signing certificate: %w", err) + } + + return csc, nil } func (e EJBCA) GetRootChain(_ context.Context) ([]*x509.Certificate, error) { @@ -184,9 +205,13 @@ func (e EJBCA) GetRootChain(_ context.Context) ([]*x509.Certificate, error) { if e.rootCAPath != "" { caCert, err := os.ReadFile(e.rootCAPath) if err != nil { - return nil, fmt.Errorf("error reading root CA: %w", err) + return nil, errorsx.Errorf("error reading root CA: %w", err) + } + certs, err := cryptoutils.LoadCertificatesFromPEM(bytes.NewReader(caCert)) + if err != nil { + return nil, errorsx.Errorf("error loading root CA: %w", err) } - return cryptoutils.LoadCertificatesFromPEM(bytes.NewReader(caCert)) + return certs, nil } return nil, nil } diff --git a/app/controlplane/pkg/ca/ejbca/ejbca_test.go b/app/controlplane/pkg/ca/ejbca/ejbca_test.go new file mode 100644 index 000000000..21bbeb7df --- /dev/null +++ b/app/controlplane/pkg/ca/ejbca/ejbca_test.go @@ -0,0 +1,30 @@ +// +// Copyright 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. +// You may obtain a copy of the License at +// +// http://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 ejbca + +import ( + "testing" + + "github.com/chainloop-dev/chainloop/pkg/errorsx" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewInvalidArguments(t *testing.T) { + _, err := New("", "", "", "", "", "", "") + require.EqualError(t, err, "ejbca: invalid arguments") + assert.True(t, errorsx.HasStack(err)) +} diff --git a/app/controlplane/pkg/ca/fileca/fileca.go b/app/controlplane/pkg/ca/fileca/fileca.go index 3a996627b..82368cd46 100644 --- a/app/controlplane/pkg/ca/fileca/fileca.go +++ b/app/controlplane/pkg/ca/fileca/fileca.go @@ -1,5 +1,5 @@ // -// Copyright 2024 The Chainloop Authors. +// Copyright 2024-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. @@ -20,11 +20,10 @@ import ( "context" "crypto" "crypto/x509" - "errors" - "fmt" "os" "path/filepath" + "github.com/chainloop-dev/chainloop/pkg/errorsx" fulcioca "github.com/sigstore/fulcio/pkg/ca" "github.com/sigstore/fulcio/pkg/ca/baseca" "github.com/sigstore/fulcio/pkg/identity" @@ -50,7 +49,7 @@ func New(certPath, keyPath, keyPass string, verify bool) (*FileCA, error) { // if the CA is a signer, verify the chain chain, signer := baseCA.GetSignerWithChain() if err := fulcioca.VerifyCertChain(chain, signer); err != nil { - return nil, err + return nil, errorsx.Errorf("fileca: verifying certificate chain: %w", err) } } @@ -68,35 +67,40 @@ func loadKeyPair(certPath, keyPath, keyPass string) (*fulcioca.SignerCerts, erro data, err := os.ReadFile(filepath.Clean(certPath)) if err != nil { - return nil, err + return nil, errorsx.Errorf("fileca: reading certificate: %w", err) } certs, err = cryptoutils.LoadCertificatesFromPEM(bytes.NewReader(data)) if err != nil { - return nil, err + return nil, errorsx.Errorf("fileca: loading certificate: %w", err) } opaqueKey, err := pemutil.Read(keyPath, pemutil.WithPassword([]byte(keyPass))) if err != nil { - return nil, err + return nil, errorsx.Errorf("fileca: reading private key: %w", err) } var ok bool key, ok = opaqueKey.(crypto.Signer) if !ok { - return nil, errors.New(`fileca: loaded private key can't be used to sign`) + return nil, errorsx.New(`fileca: loaded private key can't be used to sign`) } return &fulcioca.SignerCerts{Certs: certs, Signer: key}, nil } func (f FileCA) CreateCertificateFromCSR(ctx context.Context, principal identity.Principal, csr *x509.CertificateRequest) (*fulcioca.CodeSigningCertificate, error) { - return f.ca.CreateCertificate(ctx, principal, csr.PublicKey) + cert, err := f.ca.CreateCertificate(ctx, principal, csr.PublicKey) + if err != nil { + return nil, errorsx.Errorf("fileca: creating certificate: %w", err) + } + + return cert, nil } func (f FileCA) GetRootChain(ctx context.Context) ([]*x509.Certificate, error) { tb, err := f.ca.TrustBundle(ctx) if err != nil { - return nil, fmt.Errorf("failed to load trust bundle: %w", err) + return nil, errorsx.Errorf("failed to load trust bundle: %w", err) } return tb[0], nil } diff --git a/app/controlplane/pkg/ca/fileca/fileca_test.go b/app/controlplane/pkg/ca/fileca/fileca_test.go new file mode 100644 index 000000000..343b8b291 --- /dev/null +++ b/app/controlplane/pkg/ca/fileca/fileca_test.go @@ -0,0 +1,51 @@ +// +// Copyright 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. +// You may obtain a copy of the License at +// +// http://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 fileca + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/chainloop-dev/chainloop/pkg/errorsx" + "github.com/chainloop-dev/chainloop/pkg/errorsx/errorsxtest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewErrorsRecordOriginStack(t *testing.T) { + invalidPEM := filepath.Join(t.TempDir(), "invalid.pem") + require.NoError(t, os.WriteFile(invalidPEM, []byte("not a pem"), 0o600)) + + testCases := []struct { + name string + certPath string + wantErr string + }{ + {name: "missing certificate", certPath: filepath.Join(t.TempDir(), "missing.pem"), wantErr: "fileca: reading certificate"}, + {name: "invalid certificate", certPath: invalidPEM, wantErr: "fileca: loading certificate"}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + _, err := New(tc.certPath, "key.pem", "", false) + require.ErrorContains(t, err, tc.wantErr) + assert.True(t, errorsx.HasStack(err)) + assert.True(t, strings.HasSuffix(errorsxtest.ErrorOriginFunction(err), "loadKeyPair")) + }) + } +} diff --git a/app/controlplane/pkg/policies/policyprovider.go b/app/controlplane/pkg/policies/policyprovider.go index 6a043aa5c..88918adb0 100644 --- a/app/controlplane/pkg/policies/policyprovider.go +++ b/app/controlplane/pkg/policies/policyprovider.go @@ -25,6 +25,7 @@ import ( schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/unmarshal" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/chainloop-dev/chainloop/pkg/grpcconn" "github.com/chainloop-dev/chainloop/pkg/policies" "google.golang.org/protobuf/encoding/protojson" @@ -93,6 +94,29 @@ var ( ErrUnauthorized = fmt.Errorf("unauthorized request to policy provider") ) +// UnexpectedStatusError is returned when the policy provider answers with an +// HTTP status that the client does not handle. It has its own type so that +// Sentry does not group it with transport or decode failures. +type UnexpectedStatusError struct { + // StatusCode is the HTTP status code of the response. + StatusCode int +} + +func (e *UnexpectedStatusError) Error() string { + return fmt.Sprintf("expected status code 200 but got %d", e.StatusCode) +} + +// doRequest sends req to the policy provider. A transport failure records the +// stack of the caller, so Sentry groups it by the request that failed. +func doRequest(req *http.Request) (*http.Response, error) { + resp, err := http.DefaultClient.Do(req) + if err != nil { + return nil, errorsx.Errorf("error executing policy request: %w", err) + } + + return resp, nil +} + // ResolveOption configures a policy or policy-group resolution request. type ResolveOption func(*resolveOptions) @@ -186,9 +210,9 @@ func (p *PolicyProvider) ValidateAttachment(att *schemaapi.PolicyAttachment, aut } // make the request - resp, err := http.DefaultClient.Do(req) + resp, err := doRequest(req) if err != nil { - return fmt.Errorf("error executing policy request: %w", err) + return err } if resp.StatusCode != http.StatusOK { @@ -197,15 +221,15 @@ func (p *PolicyProvider) ValidateAttachment(att *schemaapi.PolicyAttachment, aut // Ignore endpoint not found as it might not be implemented by the provider return nil case http.StatusUnauthorized, http.StatusForbidden: - return fmt.Errorf("%w: %s", ErrUnauthorized, readBodyMsg(resp)) + return errorsx.Errorf("%w: %s", ErrUnauthorized, readBodyMsg(resp)) default: - return fmt.Errorf("expected status code 200 but got %d", resp.StatusCode) + return errorsx.WithStack(&UnexpectedStatusError{StatusCode: resp.StatusCode}) } } resBytes, err := io.ReadAll(resp.Body) if err != nil { - return fmt.Errorf("error reading validation response: %w", err) + return errorsx.Errorf("error reading validation response: %w", err) } defer resp.Body.Close() @@ -213,7 +237,7 @@ func (p *PolicyProvider) ValidateAttachment(att *schemaapi.PolicyAttachment, aut // unmarshall response var response ValidateResponse if err := json.Unmarshal(resBytes, &response); err != nil { - return fmt.Errorf("error unmarshalling validation response: %w", err) + return errorsx.Errorf("error unmarshalling validation response: %w", err) } if !response.Valid { @@ -285,9 +309,9 @@ func (p *PolicyProvider) queryProvider(url *url.URL, digest, orgName string, aut } // make the request - resp, err := http.DefaultClient.Do(req) + resp, err := doRequest(req) if err != nil { - return "", "", fmt.Errorf("error executing policy request: %w", err) + return "", "", err } if resp.StatusCode != http.StatusOK { @@ -295,27 +319,27 @@ func (p *PolicyProvider) queryProvider(url *url.URL, digest, orgName string, aut case http.StatusNotFound: return "", "", ErrNotFound case http.StatusUnauthorized, http.StatusForbidden: - return "", "", fmt.Errorf("%w: %s", ErrUnauthorized, readBodyMsg(resp)) + return "", "", errorsx.Errorf("%w: %s", ErrUnauthorized, readBodyMsg(resp)) default: - return "", "", fmt.Errorf("expected status code 200 but got %d", resp.StatusCode) + return "", "", errorsx.WithStack(&UnexpectedStatusError{StatusCode: resp.StatusCode}) } } resBytes, err := io.ReadAll(resp.Body) if err != nil { - return "", "", fmt.Errorf("error reading policy response: %w", err) + return "", "", errorsx.Errorf("error reading policy response: %w", err) } // unmarshall response var response ProviderResponse if err := json.Unmarshal(resBytes, &response); err != nil { - return "", "", fmt.Errorf("error unmarshalling policy response: %w", err) + return "", "", errorsx.Errorf("error unmarshalling policy response: %w", err) } // if raw message is provided, just interpret it as a base64 encoded string if response.Raw != nil { if err := unmarshalFromRaw(response.Raw, out); err != nil { - return "", "", fmt.Errorf("error unmarshalling policy response: %w", err) + return "", "", errorsx.Errorf("error unmarshalling policy response: %w", err) } } else if response.Data != nil { // extract the policy payload from the query response @@ -325,7 +349,7 @@ func (p *PolicyProvider) queryProvider(url *url.URL, digest, orgName string, aut } if err := protojson.Unmarshal(jsonPolicy, out); err != nil { - return "", "", fmt.Errorf("error unmarshalling policy response: %w", err) + return "", "", errorsx.Errorf("error unmarshalling policy response: %w", err) } } diff --git a/app/controlplane/pkg/policies/policyprovider_http_test.go b/app/controlplane/pkg/policies/policyprovider_http_test.go index dc1dd86eb..1bea8a533 100644 --- a/app/controlplane/pkg/policies/policyprovider_http_test.go +++ b/app/controlplane/pkg/policies/policyprovider_http_test.go @@ -16,6 +16,7 @@ package policies import ( + "errors" "fmt" "net/http" "net/http/httptest" @@ -23,6 +24,7 @@ import ( "testing" schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -99,14 +101,34 @@ func TestResolveHTTPStatusHandling(t *testing.T) { assert.NotErrorIs(t, err, ErrUnauthorized) assert.NotErrorIs(t, err, ErrNotFound) assert.Contains(t, err.Error(), fmt.Sprintf("got %d", tc.statusCode)) + + var statusErr *UnexpectedStatusError + require.ErrorAs(t, err, &statusErr) + assert.Equal(t, tc.statusCode, statusErr.StatusCode) } if tc.wantMsg != "" { assert.Contains(t, err.Error(), tc.wantMsg) } + if !errors.Is(tc.wantErr, ErrNotFound) { + assert.True(t, errorsx.HasStack(err), "a provider failure must record a stack") + } }) } } +func TestResolveTransportFailureRecordsStack(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + // Close the server so that the request fails in the transport. + server.Close() + + provider := &PolicyProvider{name: "test", url: server.URL} + + _, _, err := provider.Resolve("test-policy", "", ProviderAuthOpts{Token: "test-token"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "error executing policy request") + assert.True(t, errorsx.HasStack(err)) +} + func TestResolveGroupHTTPStatusHandling(t *testing.T) { testCases := []struct { name string diff --git a/app/controlplane/plugins/core/dependency-track/v1/client/sbom.go b/app/controlplane/plugins/core/dependency-track/v1/client/sbom.go index f58a726b2..7840f5807 100644 --- a/app/controlplane/plugins/core/dependency-track/v1/client/sbom.go +++ b/app/controlplane/plugins/core/dependency-track/v1/client/sbom.go @@ -27,6 +27,8 @@ import ( "strings" "encoding/json" + + "github.com/chainloop-dev/chainloop/pkg/errorsx" ) type base struct { @@ -222,18 +224,17 @@ func teamPermissionsRequest(httpClient *http.Client, host *url.URL, apiKey strin // Check the response if res.StatusCode != http.StatusOK { - err = fmt.Errorf("bad status: %s", res.Status) - return nil, err + return nil, newStatusError(res) } resBody, err := io.ReadAll(res.Body) if err != nil { - return nil, err + return nil, errorsx.Errorf("reading the response body: %w", err) } resp := &teamPermissionsResponse{} if err := json.Unmarshal(resBody, resp); err != nil { - return nil, err + return nil, errorsx.Errorf("decoding the team permissions response: %w", err) } return resp, nil @@ -286,18 +287,17 @@ func uploadSBOMRequest(httpClient *http.Client, host *url.URL, apiKey string, va // Check the response if res.StatusCode != http.StatusOK { - err = fmt.Errorf("bad status: %s", res.Status) - return nil, err + return nil, newStatusError(res) } resBody, err := io.ReadAll(res.Body) if err != nil { - return nil, err + return nil, errorsx.Errorf("reading the response body: %w", err) } resp := &uploadSBOMResponse{} if err := json.Unmarshal(resBody, resp); err != nil { - return nil, err + return nil, errorsx.Errorf("decoding the SBOM upload response: %w", err) } return resp, nil @@ -327,6 +327,23 @@ func projectExists(httpClient *http.Client, host *url.URL, apiKey string, projec case http.StatusNotFound: return false, nil default: - return false, fmt.Errorf("bad status: %s", res.Status) + return false, newStatusError(res) } } + +// StatusError is returned when Dependency-Track answers with an unexpected +// HTTP status. It has its own type so that Sentry does not group it with +// transport or decode failures. +type StatusError struct { + // Status is the HTTP status line of the response, for example "403 Forbidden". + Status string + // StatusCode is the HTTP status code of the response. + StatusCode int +} + +func (e *StatusError) Error() string { return "bad status: " + e.Status } + +// newStatusError returns a StatusError for res with the stack of the caller. +func newStatusError(res *http.Response) error { + return errorsx.WithStack(&StatusError{Status: res.Status, StatusCode: res.StatusCode}) +} diff --git a/app/controlplane/plugins/core/dependency-track/v1/client/sbom_test.go b/app/controlplane/plugins/core/dependency-track/v1/client/sbom_test.go index d0b1ddecd..678843a4f 100644 --- a/app/controlplane/plugins/core/dependency-track/v1/client/sbom_test.go +++ b/app/controlplane/plugins/core/dependency-track/v1/client/sbom_test.go @@ -19,10 +19,13 @@ import ( "bytes" "io" "net/http" + "net/http/httptest" "net/url" "testing" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) const hostname = "http://example.com" @@ -135,3 +138,52 @@ func TestDoCheck(t *testing.T) { } } } + +func TestBadStatusIsTypedWithStack(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusForbidden) + })) + defer server.Close() + + host, err := url.Parse(server.URL) + require.NoError(t, err) + + testCases := []struct { + name string + call func() error + }{ + { + name: "team permissions", + call: func() error { + _, err := teamPermissionsRequest(server.Client(), host, "key") + return err + }, + }, + { + name: "upload SBOM", + call: func() error { + _, err := uploadSBOMRequest(server.Client(), host, "key", map[string]io.Reader{}) + return err + }, + }, + { + name: "project exists", + call: func() error { + _, err := projectExists(server.Client(), host, "key", "id") + return err + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := tc.call() + + var statusErr *StatusError + require.ErrorAs(t, err, &statusErr) + assert.Equal(t, http.StatusForbidden, statusErr.StatusCode) + assert.EqualError(t, err, "bad status: 403 Forbidden") + assert.True(t, errorsx.HasStack(err)) + }) + } +} diff --git a/app/controlplane/plugins/core/discord-webhook/v1/discord.go b/app/controlplane/plugins/core/discord-webhook/v1/discord.go index 5a1283327..048a2bfbf 100644 --- a/app/controlplane/plugins/core/discord-webhook/v1/discord.go +++ b/app/controlplane/plugins/core/discord-webhook/v1/discord.go @@ -26,6 +26,7 @@ import ( "net/http" "github.com/chainloop-dev/chainloop/app/controlplane/plugins/sdk/v1" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/go-kratos/kratos/v2/log" ) @@ -108,12 +109,12 @@ func (i *Integration) Register(ctx context.Context, req *sdk.RegistrationRequest } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("invalid webhook URL, status: %s", resp.Status) + return nil, errorsx.Errorf("invalid webhook URL, status: %s", resp.Status) } var webHookInfo webhookResponse if err := json.NewDecoder(resp.Body).Decode(&webHookInfo); err != nil { - return nil, fmt.Errorf("invalid webhook URL: %w", err) + return nil, errorsx.Errorf("invalid webhook URL: %w", err) } // Configuration State @@ -236,7 +237,7 @@ func executeWebhook(client *http.Client, webhookURL, usernameOverride string, st if r.StatusCode != http.StatusOK { b, _ := io.ReadAll(r.Body) - return fmt.Errorf("non-OK HTTP status while calling the webhook: %d, body: %s", r.StatusCode, string(b)) + return errorsx.Errorf("non-OK HTTP status while calling the webhook: %d, body: %s", r.StatusCode, string(b)) } return nil diff --git a/app/controlplane/plugins/core/guac/v1/guac.go b/app/controlplane/plugins/core/guac/v1/guac.go index b31ed61ec..fd90ee902 100644 --- a/app/controlplane/plugins/core/guac/v1/guac.go +++ b/app/controlplane/plugins/core/guac/v1/guac.go @@ -29,6 +29,7 @@ import ( "code.cloudfoundry.org/bytefmt" schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" "github.com/chainloop-dev/chainloop/app/controlplane/plugins/sdk/v1" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/go-kratos/kratos/v2/log" "google.golang.org/api/option" ) @@ -124,7 +125,7 @@ func (i *Integration) Register(ctx context.Context, req *sdk.RegistrationRequest // Check that the credentials are valid and the bucket exists client, err := storage.NewClient(ctx, option.WithCredentialsJSON([]byte(request.Credentials))) if err != nil { - return nil, fmt.Errorf("creating storage client: %w", err) + return nil, errorsx.Errorf("creating storage client: %w", err) } _, err = loadBucket(ctx, client, request.Bucket) @@ -175,7 +176,7 @@ func (i *Integration) Execute(ctx context.Context, req *sdk.ExecutionRequest) er client, err := storage.NewClient(ctx, option.WithCredentialsJSON([]byte(req.RegistrationInfo.Credentials.Password))) if err != nil { - return fmt.Errorf("creating storage client: %w", err) + return errorsx.Errorf("creating storage client: %w", err) } bucket, err := loadBucket(ctx, client, registrationConfig.Bucket) @@ -228,7 +229,7 @@ const pathPrefix = "chainloop" func uploadToBucket(ctx context.Context, bucket *storage.BucketHandle, filename string, content []byte, md *sdk.ChainloopMetadata, logger *log.Helper) error { bucketInfo, err := bucket.Attrs(ctx) if err != nil { - return fmt.Errorf("getting bucket info: %w", err) + return errorsx.Errorf("getting bucket info: %w", err) } buf := bytes.NewBuffer(content) @@ -255,7 +256,7 @@ func uploadToBucket(ctx context.Context, bucket *storage.BucketHandle, filename } if _, err := io.Copy(w, buf); err != nil { - return fmt.Errorf("writing to the bucket: %w", err) + return errorsx.Errorf("writing to the bucket: %w", err) } logger.Infow("msg", "uploaded to the bucket", "file", filename, "bucket", bucketInfo.Name, "size", bytefmt.ByteSize(fileSize)) @@ -280,13 +281,13 @@ func loadBucket(ctx context.Context, client *storage.Client, bucket string) (*st return nil, fmt.Errorf("the bucket %s does not exist", bucket) } - return nil, fmt.Errorf("checking the bucket: %w", err) + return nil, errorsx.Errorf("checking the bucket: %w", err) } // Write test file to make sure credentials have write permissions testFile := b.Object("chainloop-test-write") if err := testFile.NewWriter(ctx).Close(); err != nil { - return nil, fmt.Errorf("we can't write to the bucket %s: %w", bucket, err) + return nil, errorsx.Errorf("we can't write to the bucket %s: %w", bucket, err) } _ = testFile.Delete(ctx) diff --git a/app/controlplane/plugins/core/slack-webhook/v1/slack_webhook.go b/app/controlplane/plugins/core/slack-webhook/v1/slack_webhook.go index ecca50ae9..59e1935d2 100644 --- a/app/controlplane/plugins/core/slack-webhook/v1/slack_webhook.go +++ b/app/controlplane/plugins/core/slack-webhook/v1/slack_webhook.go @@ -27,6 +27,7 @@ import ( "text/template" "github.com/chainloop-dev/chainloop/app/controlplane/plugins/sdk/v1" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/go-kratos/kratos/v2/log" ) @@ -154,7 +155,7 @@ func executeWebhook(client *http.Client, webhookURL, msgContent string) error { if r.StatusCode != http.StatusOK { b, _ := io.ReadAll(r.Body) - return fmt.Errorf("non-OK HTTP status while calling the webhook: %d, body: %s", r.StatusCode, string(b)) + return errorsx.Errorf("non-OK HTTP status while calling the webhook: %d, body: %s", r.StatusCode, string(b)) } return nil diff --git a/app/controlplane/plugins/core/smtp/v1/extension.go b/app/controlplane/plugins/core/smtp/v1/extension.go index 6f0a2f6de..f06ae69e6 100644 --- a/app/controlplane/plugins/core/smtp/v1/extension.go +++ b/app/controlplane/plugins/core/smtp/v1/extension.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. @@ -22,6 +22,7 @@ import ( nsmtp "net/smtp" "github.com/chainloop-dev/chainloop/app/controlplane/plugins/sdk/v1" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/go-kratos/kratos/v2/log" ) @@ -204,7 +205,7 @@ func sendEmail(host string, port string, user, password, from, to, cc, subject, auth := nsmtp.PlainAuth("", user, password, host) err := nsmtp.SendMail(host+":"+port, auth, from, []string{to}, []byte(message)) if err != nil { - return fmt.Errorf("error sending email: %w", err) + return errorsx.Errorf("error sending email: %w", err) } return nil diff --git a/app/controlplane/plugins/core/webhook/v1/webhook.go b/app/controlplane/plugins/core/webhook/v1/webhook.go index f3059c666..55d6a7174 100644 --- a/app/controlplane/plugins/core/webhook/v1/webhook.go +++ b/app/controlplane/plugins/core/webhook/v1/webhook.go @@ -27,6 +27,7 @@ import ( schemaapi "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" "github.com/chainloop-dev/chainloop/app/controlplane/plugins/sdk/v1" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/go-kratos/kratos/v2/log" ) @@ -278,7 +279,7 @@ func (i *Integration) sendWebhook(ctx context.Context, url, kind string, payload if resp.StatusCode < 200 || resp.StatusCode >= 300 { i.Logger.Errorw("webhook responded with non-success status code", "status", resp.StatusCode, "body", string(body)) - return fmt.Errorf("webhook responded with status code %d: %s", resp.StatusCode, string(body)) + return errorsx.Errorf("webhook responded with status code %d: %s", resp.StatusCode, string(body)) } return nil diff --git a/app/controlplane/plugins/sdk/v1/httpclient.go b/app/controlplane/plugins/sdk/v1/httpclient.go index 7aa8430de..5aad086db 100644 --- a/app/controlplane/plugins/sdk/v1/httpclient.go +++ b/app/controlplane/plugins/sdk/v1/httpclient.go @@ -20,6 +20,7 @@ import ( "net/http" "time" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/chainloop-dev/chainloop/pkg/netguard" ) @@ -85,5 +86,23 @@ func NewHTTPClient(opts HTTPClientOptions) *http.Client { netguard.RestrictTransport(transport) } - return &http.Client{Timeout: opts.Timeout, Transport: transport} + return &http.Client{Timeout: opts.Timeout, Transport: &stackTransport{base: transport}} +} + +// stackTransport records the call stack of the plugin code that sent a +// request when the request fails, so Sentry groups a failed outbound call by +// the code that made it and not by the place that reports the error. +type stackTransport struct { + // base is the transport that sends the requests. + base http.RoundTripper +} + +// RoundTrip sends req with the base transport and adds a stack to its error. +func (t *stackTransport) RoundTrip(req *http.Request) (*http.Response, error) { + resp, err := t.base.RoundTrip(req) + if err != nil { + return nil, errorsx.WithStack(err) + } + + return resp, nil } diff --git a/app/controlplane/plugins/sdk/v1/httpclient_test.go b/app/controlplane/plugins/sdk/v1/httpclient_test.go index 2d0980510..c9d50ef53 100644 --- a/app/controlplane/plugins/sdk/v1/httpclient_test.go +++ b/app/controlplane/plugins/sdk/v1/httpclient_test.go @@ -20,6 +20,7 @@ import ( "net/http/httptest" "testing" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -48,6 +49,7 @@ func TestNewHTTPClientPublicTargetsOnly(t *testing.T) { resp, err := client.Get(server.URL) if tc.wantBlocked { require.ErrorIs(t, err, ErrBlockedTarget) + assert.True(t, errorsx.HasStack(err), "a failed request must record a stack") return } @@ -76,7 +78,9 @@ func TestNewHTTPClientProxyUse(t *testing.T) { t.Run(tc.name, func(t *testing.T) { client := NewHTTPClient(HTTPClientOptions{PublicTargetsOnly: tc.publicTargetsOnly}) - transport, ok := client.Transport.(*http.Transport) + st, ok := client.Transport.(*stackTransport) + require.True(t, ok) + transport, ok := st.base.(*http.Transport) require.True(t, ok) if tc.wantProxy { diff --git a/pkg/blobmanager/azureblob/backend.go b/pkg/blobmanager/azureblob/backend.go index edc3f72b4..9a16c86a4 100644 --- a/pkg/blobmanager/azureblob/backend.go +++ b/pkg/blobmanager/azureblob/backend.go @@ -17,7 +17,6 @@ package azureblob import ( "context" - "errors" "fmt" "io" "os" @@ -29,6 +28,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/bloberror" pb "github.com/chainloop-dev/chainloop/app/artifact-cas/api/cas/v1" backend "github.com/chainloop-dev/chainloop/pkg/blobmanager" + "github.com/chainloop-dev/chainloop/pkg/errorsx" ) type Backend struct { @@ -43,7 +43,7 @@ var _ backend.UploaderDownloader = (*Backend)(nil) func NewBackend(creds *Credentials) (*Backend, error) { credential, err := azidentity.NewClientSecretCredential(creds.TenantID, creds.ClientID, creds.ClientSecret, nil) if err != nil { - return nil, fmt.Errorf("failed to create Azure Service principal Credential: %w", err) + return nil, errorsx.Errorf("failed to create Azure Service principal Credential: %w", err) } return &Backend{ @@ -60,7 +60,7 @@ func (b *Backend) client() (*azblob.Client, error) { // Top level client client, err := azblob.NewClient(url, b.credentials, nil) if err != nil { - return nil, fmt.Errorf("failed to create Azure Blob Storage client: %w", err) + return nil, errorsx.Errorf("failed to create Azure Blob Storage client: %w", err) } return client, nil @@ -78,7 +78,7 @@ func (b *Backend) getServiceURL() string { func (b *Backend) blobClient(digest string) (*blob.Client, error) { blobClient, err := blob.NewClient(b.resourcePath(digest), b.credentials, nil) if err != nil { - return nil, fmt.Errorf("failed to create Azure Blob Storage client: %w", err) + return nil, errorsx.Errorf("failed to create Azure Blob Storage client: %w", err) } return blobClient, nil @@ -135,14 +135,14 @@ func (b *Backend) Upload(ctx context.Context, r io.Reader, resource *pb.CASResou _, err = client.UploadFile(ctx, b.container, resourceName(resource.Digest), f, &azblob.UploadFileOptions{ Metadata: metadata, }) - return err + return errorsx.WithStack(err) } _, err = client.UploadStream(ctx, b.container, resourceName(resource.Digest), r, &azblob.UploadStreamOptions{ Metadata: metadata, }) - return err + return errorsx.WithStack(err) } func (b *Backend) Describe(ctx context.Context, digest string) (*pb.CASResource, error) { @@ -157,19 +157,19 @@ func (b *Backend) Describe(ctx context.Context, digest string) (*pb.CASResource, return nil, backend.NewErrNotFound("artifact") } - return nil, fmt.Errorf("failed to get blob properties: %w", err) + return nil, errorsx.Errorf("failed to get blob properties: %w", err) } // Check asset author is chainloop that way we can ignore files uploaded by other tools // note: this is not a security mechanism, an additional check will be put in place for tamper check author, ok := properties.Metadata[annotationNameAuthor] if !ok || *author != backend.AuthorAnnotation { - return nil, errors.New("asset not uploaded by Chainloop") + return nil, errorsx.New("asset not uploaded by Chainloop") } fileName, ok := properties.Metadata[annotationNameFilename] if !ok { - return nil, fmt.Errorf("couldn't find file metadata") + return nil, errorsx.Errorf("couldn't find file metadata") } return &pb.CASResource{ @@ -181,7 +181,7 @@ func (b *Backend) Describe(ctx context.Context, digest string) (*pb.CASResource, func (b *Backend) Download(ctx context.Context, w io.Writer, digest string) error { exists, err := b.Exists(ctx, digest) if err != nil { - return fmt.Errorf("invalid artifact: %w", err) + return errorsx.Errorf("invalid artifact: %w", err) } else if !exists { return backend.NewErrNotFound("artifact") } @@ -194,14 +194,14 @@ func (b *Backend) Download(ctx context.Context, w io.Writer, digest string) erro // Download the blob downloadStream, err := client.DownloadStream(ctx, b.container, resourceName(digest), nil) if err != nil { - return fmt.Errorf("failed to download blob: %w", err) + return errorsx.Errorf("failed to download blob: %w", err) } retryReader := downloadStream.NewRetryReader(ctx, &azblob.RetryReaderOptions{}) defer retryReader.Close() if _, err := io.Copy(w, retryReader); err != nil { - return fmt.Errorf("failed to copy blob to writer: %w", err) + return errorsx.Errorf("failed to copy blob to writer: %w", err) } return nil @@ -222,7 +222,7 @@ func (b *Backend) CheckWritePermissions(ctx context.Context) error { // Touch a file _, err = client.UploadBuffer(ctx, b.container, "healthCheck", nil, nil) if err != nil { - return fmt.Errorf("failed write to Blob Storage: %w", err) + return errorsx.Errorf("failed write to Blob Storage: %w", err) } return nil @@ -232,7 +232,7 @@ func (b *Backend) createContainer(ctx context.Context, client *azblob.Client) er // Create container name _, err := client.CreateContainer(ctx, b.container, nil) if err != nil && !bloberror.HasCode(err, bloberror.ContainerAlreadyExists) { - return fmt.Errorf("failed to create Blob storage Container: %w", err) + return errorsx.Errorf("failed to create Blob storage Container: %w", err) } return nil diff --git a/pkg/blobmanager/oci/backend.go b/pkg/blobmanager/oci/backend.go index 53b266e35..7d96a018c 100644 --- a/pkg/blobmanager/oci/backend.go +++ b/pkg/blobmanager/oci/backend.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. @@ -36,6 +36,7 @@ import ( pb "github.com/chainloop-dev/chainloop/app/artifact-cas/api/cas/v1" backend "github.com/chainloop-dev/chainloop/pkg/blobmanager" + "github.com/chainloop-dev/chainloop/pkg/errorsx" ) type Backend struct { @@ -78,12 +79,12 @@ func NewBackend(repository string, regOpts *RegistryOptions, opts ...NewBackendO // This method is very naive so signatures will be used in future releases func (b *Backend) Exists(_ context.Context, digest string) (bool, error) { if digest == "" { - return false, errors.New("digest is empty") + return false, errorsx.New("digest is empty") } ref, err := name.ParseReference(b.resourcePath(digest)) if err != nil { - return false, err + return false, errorsx.WithStack(err) } // It's not trivial to catch if the error is a 404 (yeah I know...) so we will assume that @@ -110,12 +111,12 @@ func (b *Backend) Upload(_ context.Context, r io.Reader, resource *pb.CASResourc // TODO: Split content in multiple layers and do concurrent uploads/downloads data, err := io.ReadAll(r) if err != nil { - return fmt.Errorf("reading content: %w", err) + return errorsx.Errorf("reading content: %w", err) } ref, err := name.ParseReference(b.resourcePath(resource.Digest)) if err != nil { - return fmt.Errorf("parsing reference: %w", err) + return errorsx.Errorf("parsing reference: %w", err) } img, err := craftImage(data, resource) @@ -129,7 +130,7 @@ func (b *Backend) Upload(_ context.Context, r io.Reader, resource *pb.CASResourc err = remote.Write(ref, img, remote.WithAuthFromKeychain(b.keychain)) if err != nil { - return fmt.Errorf("writing image: %w", err) + return errorsx.Errorf("writing image: %w", err) } return nil @@ -141,11 +142,11 @@ func (b *Backend) resourcePath(resourceName string) string { func craftImage(content []byte, resource *pb.CASResource) (v1.Image, error) { if len(content) == 0 { - return nil, errors.New("content is empty") + return nil, errorsx.New("content is empty") } if resource == nil || resource.FileName == "" || resource.Digest == "" { - return nil, errors.New("resource metadata is not valid") + return nil, errorsx.New("resource metadata is not valid") } base := mutate.MediaType(empty.Image, types.OCIManifestSchema1) @@ -159,7 +160,7 @@ func craftImage(content []byte, resource *pb.CASResource) (v1.Image, error) { layer := static.NewLayer(content, backend.DetectedMediaType(content)) img, err := mutate.Append(base, mutate.Addendum{Layer: layer}) if err != nil { - return nil, err + return nil, errorsx.WithStack(err) } return img, nil @@ -167,12 +168,12 @@ func craftImage(content []byte, resource *pb.CASResource) (v1.Image, error) { func (b *Backend) Describe(_ context.Context, digest string) (*pb.CASResource, error) { if digest == "" { - return nil, errors.New("digest is empty") + return nil, errorsx.New("digest is empty") } ref, err := name.ParseReference(b.resourcePath(digest)) if err != nil { - return nil, fmt.Errorf("parsing reference: %w", err) + return nil, errorsx.Errorf("parsing reference: %w", err) } img, err := remote.Image(ref, remote.WithAuthFromKeychain(b.keychain)) @@ -182,7 +183,7 @@ func (b *Backend) Describe(_ context.Context, digest string) (*pb.CASResource, e return nil, backend.NewErrNotFound("image") } - return nil, fmt.Errorf("getting image: %w", err) + return nil, errorsx.Errorf("getting image: %w", err) } if err := validateImage(img, digest); err != nil { @@ -191,7 +192,7 @@ func (b *Backend) Describe(_ context.Context, digest string) (*pb.CASResource, e manifest, err := img.Manifest() if err != nil { - return nil, fmt.Errorf("extracting manifest: %w", err) + return nil, errorsx.Errorf("extracting manifest: %w", err) } // Validate image already checked that the manifest has exactly one layer @@ -199,7 +200,7 @@ func (b *Backend) Describe(_ context.Context, digest string) (*pb.CASResource, e filename, ok := manifest.Annotations[ocispec.AnnotationTitle] if !ok { - return nil, errors.New("couldn't find file metadata") + return nil, errorsx.New("couldn't find file metadata") } return &pb.CASResource{Digest: digest, FileName: filename, Size: size}, nil @@ -207,17 +208,17 @@ func (b *Backend) Describe(_ context.Context, digest string) (*pb.CASResource, e func (b *Backend) Download(_ context.Context, w io.Writer, digest string) error { if digest == "" { - return errors.New("digest is empty") + return errorsx.New("digest is empty") } ref, err := name.ParseReference(b.resourcePath(digest)) if err != nil { - return fmt.Errorf("parsing reference: %w", err) + return errorsx.Errorf("parsing reference: %w", err) } img, err := remote.Image(ref, remote.WithAuthFromKeychain(b.keychain)) if err != nil { - return fmt.Errorf("getting image: %w", err) + return errorsx.Errorf("getting image: %w", err) } if err := validateImage(img, digest); err != nil { @@ -227,13 +228,13 @@ func (b *Backend) Download(_ context.Context, w io.Writer, digest string) error // Download the layer with the same digest, not relying on the image name l, err := img.LayerByDiffID(v1.Hash{Algorithm: "sha256", Hex: digest}) if err != nil { - return fmt.Errorf("getting layer with hash sha256:%s: %w", digest, err) + return errorsx.Errorf("getting layer with hash sha256:%s: %w", digest, err) } // Do not uncompress since we want the raw stored data rc, err := l.Compressed() if err != nil { - return fmt.Errorf("extracting data from layer: %w", err) + return errorsx.Errorf("extracting data from layer: %w", err) } defer rc.Close() @@ -241,7 +242,7 @@ func (b *Backend) Download(_ context.Context, w io.Writer, digest string) error buf := make([]byte, 1<<20) _, err = io.CopyBuffer(w, rc, buf) if err != nil { - return fmt.Errorf("copying data from layer: %w", err) + return errorsx.Errorf("copying data from layer: %w", err) } return nil @@ -252,35 +253,35 @@ func validateImage(img v1.Image, digest string) error { // Review required annotations m, err := img.Manifest() if err != nil { - return fmt.Errorf("getting manifest: %w", err) + return errorsx.Errorf("getting manifest: %w", err) } if v, ok := m.Annotations[ocispec.AnnotationAuthors]; !ok || v != backend.AuthorAnnotation { - return errors.New("image not uploaded by chainloop") + return errorsx.New("image not uploaded by chainloop") } if v, ok := m.Annotations[ocispec.AnnotationTitle]; !ok && v != "" { - return errors.New("image does not contain filename information") + return errorsx.New("image does not contain filename information") } // NOTE: we use img.Layers instead of LayerByDiffID because the latter does not compute the image manifest layers, err := img.Layers() if err != nil { - return fmt.Errorf("getting layers: %w", err) + return errorsx.Errorf("getting layers: %w", err) } if len(layers) != 1 { - return errors.New("image does not contain a single layer") + return errorsx.New("image does not contain a single layer") } // Check the actual layer digest content meets the expected one d, err := layers[0].Digest() if err != nil { - return fmt.Errorf("getting layer digest: %w", err) + return errorsx.Errorf("getting layer digest: %w", err) } if d.Hex != digest { - return errors.New("layer digest does not match the expected one") + return errorsx.New("layer digest does not match the expected one") } return nil @@ -290,8 +291,8 @@ func validateImage(img v1.Image, digest string) error { func (b *Backend) CheckWritePermissions(_ context.Context) error { ref, err := name.ParseReference(fmt.Sprintf("%s/chainloop-test", b.repo)) if err != nil { - return fmt.Errorf("parsing the reference image for validation: %w", err) + return errorsx.Errorf("parsing the reference image for validation: %w", err) } - return remote.CheckPushPermission(ref, b.keychain, http.DefaultTransport) + return errorsx.WithStack(remote.CheckPushPermission(ref, b.keychain, http.DefaultTransport)) } diff --git a/pkg/blobmanager/s3/backend.go b/pkg/blobmanager/s3/backend.go index f9b8c8f3e..01e8c22a3 100644 --- a/pkg/blobmanager/s3/backend.go +++ b/pkg/blobmanager/s3/backend.go @@ -33,6 +33,7 @@ import ( "github.com/aws/smithy-go" pb "github.com/chainloop-dev/chainloop/app/artifact-cas/api/cas/v1" backend "github.com/chainloop-dev/chainloop/pkg/blobmanager" + "github.com/chainloop-dev/chainloop/pkg/errorsx" ) const ( @@ -52,11 +53,11 @@ const defaultRegion = "us-east-1" func NewBackend(creds *Credentials) (*Backend, error) { if creds == nil { - return nil, errors.New("credentials cannot be nil") + return nil, errorsx.New("credentials cannot be nil") } if err := creds.Validate(); err != nil { - return nil, fmt.Errorf("invalid credentials: %w", err) + return nil, errorsx.Errorf("invalid credentials: %w", err) } // Set a default region if not provided @@ -68,7 +69,7 @@ func NewBackend(creds *Credentials) (*Backend, error) { // Bucket might contain the not only the bucket name but also the endpoint endpoint, bucket, err := extractLocationAndBucket(creds) if err != nil { - return nil, fmt.Errorf("failed to parse bucket name: %w", err) + return nil, errorsx.Errorf("failed to parse bucket name: %w", err) } // Using AWS config directly instead of using config.LoadDefaultConfig @@ -124,7 +125,7 @@ func extractLocationAndBucket(creds *Credentials) (string, string, error) { // so we override the bucket and set the endpoint if needed parsedLocation, err := url.Parse(creds.Location) if err != nil { - return "", "", fmt.Errorf("failed to parse location: %w", err) + return "", "", errorsx.Errorf("failed to parse location: %w", err) } host := parsedLocation.Host @@ -139,7 +140,7 @@ func extractLocationAndBucket(creds *Credentials) (string, string, error) { return endpoint, pathSegments[1], nil } - return "", "", fmt.Errorf("the location doesn't contain a bucket name") + return "", "", errorsx.Errorf("the location doesn't contain a bucket name") } // Exists check that the artifact is already present in the repository @@ -169,7 +170,7 @@ func (b *Backend) Upload(ctx context.Context, r io.Reader, resource *pb.CASResou // variants), and some S3-compatible endpoints such as R2 reject it outright. if _, err := uploader.Upload(ctx, input); err != nil { - return fmt.Errorf("failed to upload to bucket: %w", err) + return errorsx.Errorf("failed to upload to bucket: %w", err) } return nil @@ -196,24 +197,24 @@ func (b *Backend) Describe(ctx context.Context, digest string) (*pb.CASResource, return nil, backend.NewErrNotFound("artifact") } - return nil, fmt.Errorf("failed to read from bucket: %w", err) + return nil, errorsx.Errorf("failed to read from bucket: %w", err) } // Check integrity of the remote object if resp.ChecksumSHA256 != nil && *resp.ChecksumSHA256 != hexSha256ToBinaryB64(digest) { - return nil, fmt.Errorf("failed to validate integrity of object, got=%s, want=%s", *resp.ChecksumSHA256, hexSha256ToBinaryB64(digest)) + return nil, errorsx.Errorf("failed to validate integrity of object, got=%s, want=%s", *resp.ChecksumSHA256, hexSha256ToBinaryB64(digest)) } // Check asset author is Chainloop that way we can ignore files uploaded by other tools // note: this is not a security mechanism, an additional check will be put in place for tamper check author, ok := resp.Metadata[annotationNameAuthor] if !ok || author != backend.AuthorAnnotation { - return nil, errors.New("asset not uploaded by Chainloop") + return nil, errorsx.New("asset not uploaded by Chainloop") } fileName, ok := resp.Metadata[annotationNameFilename] if !ok { - return nil, fmt.Errorf("couldn't find file metadata") + return nil, errorsx.Errorf("couldn't find file metadata") } return &pb.CASResource{ @@ -243,7 +244,7 @@ func (b *Backend) Download(ctx context.Context, w io.Writer, digest string) erro Key: aws.String(resourceName(digest)), }) - return err + return errorsx.WithStack(err) } // CheckWritePermissions performs an actual write to the repository to check that the credentials @@ -258,7 +259,7 @@ func (b *Backend) CheckWritePermissions(ctx context.Context) error { // Write to the bucket if _, err := b.client.PutObject(ctx, input); err != nil { - return fmt.Errorf("failed to write to bucket: %w", err) + return errorsx.Errorf("failed to write to bucket: %w", err) } // and read the object back @@ -268,7 +269,7 @@ func (b *Backend) CheckWritePermissions(ctx context.Context) error { }) if err != nil { - return fmt.Errorf("failed to read from bucket: %w", err) + return errorsx.Errorf("failed to read from bucket: %w", err) } return nil diff --git a/pkg/blobmanager/s3accesspoint/backend.go b/pkg/blobmanager/s3accesspoint/backend.go index 6c30e663a..04b3d2930 100644 --- a/pkg/blobmanager/s3accesspoint/backend.go +++ b/pkg/blobmanager/s3accesspoint/backend.go @@ -35,6 +35,7 @@ import ( pb "github.com/chainloop-dev/chainloop/app/artifact-cas/api/cas/v1" robotaccount "github.com/chainloop-dev/chainloop/internal/robotaccount/cas" backend "github.com/chainloop-dev/chainloop/pkg/blobmanager" + "github.com/chainloop-dev/chainloop/pkg/errorsx" ) const ( @@ -88,7 +89,7 @@ func NewBackend(ctx context.Context, creds *Credentials) (*Backend, error) { // against the pod identity is necessary. awsCfg, err := awsconfig.LoadDefaultConfig(ctx, awsconfig.WithRegion(creds.Region)) if err != nil { - return nil, fmt.Errorf("loading aws config: %w", err) + return nil, errorsx.Errorf("loading aws config: %w", err) } stsClient := sts.NewFromConfig(awsCfg) @@ -160,7 +161,7 @@ func (b *Backend) Upload(ctx context.Context, r io.Reader, resource *pb.CASResou }, }) if err != nil { - return fmt.Errorf("uploading to access point: %w", err) + return errorsx.Errorf("uploading to access point: %w", err) } return nil } @@ -180,23 +181,23 @@ func (b *Backend) Describe(ctx context.Context, digest string) (*pb.CASResource, if errors.As(err, &apiErr) && apiErr.ErrorCode() == "NotFound" { return nil, backend.NewErrNotFound("artifact") } - return nil, fmt.Errorf("reading from access point: %w", err) + return nil, errorsx.Errorf("reading from access point: %w", err) } // Integrity check: when S3 returned a checksum, make sure the digest // the caller asked for matches the server's recorded value. if resp.ChecksumSHA256 != nil && *resp.ChecksumSHA256 != hexSha256ToBinaryB64(digest) { - return nil, fmt.Errorf("failed to validate integrity of object, got=%s, want=%s", + return nil, errorsx.Errorf("failed to validate integrity of object, got=%s, want=%s", *resp.ChecksumSHA256, hexSha256ToBinaryB64(digest)) } author, ok := resp.Metadata[annotationNameAuthor] if !ok || author != backend.AuthorAnnotation { - return nil, errors.New("asset not uploaded by Chainloop") + return nil, errorsx.New("asset not uploaded by Chainloop") } fileName, ok := resp.Metadata[annotationNameFilename] if !ok { - return nil, errors.New("couldn't find file metadata") + return nil, errorsx.New("couldn't find file metadata") } var size int64 @@ -227,7 +228,7 @@ func (b *Backend) Download(ctx context.Context, w io.Writer, digest string) erro Bucket: aws.String(b.creds.AccessPointARN), Key: aws.String(key), }) - return err + return errorsx.WithStack(err) } // CheckWritePermissions verifies that the calling org can actually mint a @@ -249,13 +250,13 @@ func (b *Backend) CheckWritePermissions(ctx context.Context) error { Bucket: aws.String(b.creds.AccessPointARN), Key: aws.String(key), }); err != nil { - return fmt.Errorf("writing healthcheck object: %w", err) + return errorsx.Errorf("writing healthcheck object: %w", err) } if _, err := b.s3Client.GetObject(ctx, &s3.GetObjectInput{ Bucket: aws.String(b.creds.AccessPointARN), Key: aws.String(key), }); err != nil { - return fmt.Errorf("reading healthcheck object: %w", err) + return errorsx.Errorf("reading healthcheck object: %w", err) } return nil } @@ -299,9 +300,10 @@ func (p *sessionCredentialsProvider) Retrieve(ctx context.Context) (aws.Credenti // SDK's default credential chain directly. if p.useAmbientForRetrieve { if p.ambientCreds == nil { - return aws.Credentials{}, errors.New("s3accesspoint: dev mode requested but no ambient credentials available") + return aws.Credentials{}, errorsx.New("s3accesspoint: dev mode requested but no ambient credentials available") } - return p.ambientCreds.Retrieve(ctx) + creds, err := p.ambientCreds.Retrieve(ctx) + return creds, errorsx.WithStack(err) } // The session policy intersects with the base role's permissions and @@ -333,10 +335,10 @@ func (p *sessionCredentialsProvider) Retrieve(ctx context.Context) (aws.Credenti out, err := p.stsClient.AssumeRole(ctx, input) if err != nil { - return aws.Credentials{}, fmt.Errorf("sts:AssumeRole for org %s: %w", info.OrgID, err) + return aws.Credentials{}, errorsx.Errorf("sts:AssumeRole for org %s: %w", info.OrgID, err) } if out.Credentials == nil { - return aws.Credentials{}, errors.New("sts:AssumeRole returned no credentials") + return aws.Credentials{}, errorsx.New("sts:AssumeRole returned no credentials") } return aws.Credentials{ diff --git a/pkg/cache/natskv.go b/pkg/cache/natskv.go index 4d508c1cd..14c8b95c7 100644 --- a/pkg/cache/natskv.go +++ b/pkg/cache/natskv.go @@ -24,6 +24,7 @@ import ( "sync" "time" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/nats-io/nats.go" "github.com/nats-io/nats.go/jetstream" ) @@ -110,7 +111,7 @@ func isRetryableInitError(err error) bool { func (c *natsKVCache[T]) initBucket() error { js, err := jetstream.New(c.conn) if err != nil { - return err + return errorsx.Errorf("cache: creating jetstream context: %w", err) } kv, err := js.CreateOrUpdateKeyValue(context.Background(), jetstream.KeyValueConfig{ @@ -122,7 +123,7 @@ func (c *natsKVCache[T]) initBucket() error { Storage: jetstream.MemoryStorage, }) if err != nil { - return err + return errorsx.Errorf("cache: creating or updating bucket %s: %w", c.bucket, err) } // NATS KV hardcodes DiscardNew; we want DiscardOld so the cache evicts @@ -148,7 +149,7 @@ func (c *natsKVCache[T]) ensureDiscardOld(js jetstream.JetStream) error { stream, err := js.Stream(ctx, streamName) if err != nil { - return fmt.Errorf("cache: failed to get backing stream %s: %w", streamName, err) + return errorsx.Errorf("cache: failed to get backing stream %s: %w", streamName, err) } cfg := stream.CachedInfo().Config @@ -158,7 +159,7 @@ func (c *natsKVCache[T]) ensureDiscardOld(js jetstream.JetStream) error { cfg.Discard = jetstream.DiscardOld if _, err := js.UpdateStream(ctx, cfg); err != nil { - return fmt.Errorf("cache: failed to set DiscardOld on stream %s: %w", streamName, err) + return errorsx.Errorf("cache: failed to set DiscardOld on stream %s: %w", streamName, err) } return nil } diff --git a/pkg/credentials/aws/secretmanager.go b/pkg/credentials/aws/secretmanager.go index 6e0f6f6a4..062fdbc26 100644 --- a/pkg/credentials/aws/secretmanager.go +++ b/pkg/credentials/aws/secretmanager.go @@ -30,6 +30,7 @@ import ( "github.com/aws/aws-sdk-go-v2/service/sso/types" "github.com/aws/smithy-go" "github.com/chainloop-dev/chainloop/pkg/credentials" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/chainloop-dev/chainloop/pkg/servicelogger" "github.com/google/uuid" @@ -57,7 +58,7 @@ type NewManagerOpts struct { func NewManager(opts *NewManagerOpts) (*Manager, error) { if opts.Region == "" || opts.AccessKey == "" || opts.SecretKey == "" { - return nil, errors.New("region, accessKey and the secretKey are required") + return nil, errorsx.New("region, accessKey and the secretKey are required") } l := opts.Logger @@ -92,7 +93,7 @@ func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any, // Store the credentials as json key pairs c, err := json.Marshal(creds) if err != nil { - return "", fmt.Errorf("marshaling credentials to be stored: %w", err) + return "", errorsx.Errorf("marshaling credentials to be stored: %w", err) } if o.SecretName != "" { @@ -117,7 +118,7 @@ func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any, } if err != nil { - return "", fmt.Errorf("saving secret in AWS: %w", err) + return "", errorsx.Errorf("saving secret in AWS: %w", err) } return secretName, nil @@ -135,11 +136,11 @@ func (m *Manager) ReadCredentials(ctx context.Context, secretID string, creds an case (&types.ResourceNotFoundException{}).ErrorCode(): return fmt.Errorf("%w: path=%s", credentials.ErrNotFound, secretID) default: - return fmt.Errorf("getting AWS Secret Value: %w", err) + return errorsx.Errorf("getting AWS Secret Value: %w", err) } } - return fmt.Errorf("getting AWS Secret Value: %w", err) + return errorsx.Errorf("getting AWS Secret Value: %w", err) } return json.Unmarshal([]byte(*resp.SecretString), creds) @@ -150,5 +151,5 @@ func (m *Manager) DeleteCredentials(ctx context.Context, secretID string) error SecretId: aws.String(secretID), }) - return err + return errorsx.WithStack(err) } diff --git a/pkg/credentials/azurekv/keyvault.go b/pkg/credentials/azurekv/keyvault.go index 6d7a1a8c3..3975c3c2e 100644 --- a/pkg/credentials/azurekv/keyvault.go +++ b/pkg/credentials/azurekv/keyvault.go @@ -29,6 +29,7 @@ import ( "github.com/chainloop-dev/chainloop/pkg/credentials" "github.com/google/uuid" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/chainloop-dev/chainloop/pkg/servicelogger" "github.com/go-kratos/kratos/v2/log" ) @@ -69,19 +70,19 @@ var ErrValidation = errors.New("credentials validation error") func (o *NewManagerOpts) Validate() error { if o.TenantID == "" { - return fmt.Errorf("%w: missing tenant ID", ErrValidation) + return errorsx.Errorf("%w: missing tenant ID", ErrValidation) } if o.ClientID == "" { - return fmt.Errorf("%w: missing client ID", ErrValidation) + return errorsx.Errorf("%w: missing client ID", ErrValidation) } if o.ClientSecret == "" { - return fmt.Errorf("%w: missing client secret", ErrValidation) + return errorsx.Errorf("%w: missing client secret", ErrValidation) } if o.VaultURI == "" { - return fmt.Errorf("%w: missing VAULT URI", ErrValidation) + return errorsx.Errorf("%w: missing VAULT URI", ErrValidation) } return nil @@ -102,7 +103,7 @@ func NewManager(opts *NewManagerOpts) (*Manager, error) { credential, err := azidentity.NewClientSecretCredential(opts.TenantID, opts.ClientID, opts.ClientSecret, nil) if err != nil { - return nil, fmt.Errorf("failed to create Azure Service principal Credential: %w", err) + return nil, errorsx.Errorf("failed to create Azure Service principal Credential: %w", err) } // Establish a connection to the Key Vault client @@ -131,11 +132,11 @@ func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any, // Store the credentials as json key pairs c, err := json.Marshal(creds) if err != nil { - return "", fmt.Errorf("marshaling credentials to be stored: %w", err) + return "", errorsx.Errorf("marshaling credentials to be stored: %w", err) } if _, err := m.client.SetSecret(ctx, secretName, azsecrets.SetSecretParameters{Value: strPtr(string(c))}, nil); err != nil { - return "", fmt.Errorf("failed to set secret: %w", err) + return "", errorsx.Errorf("failed to set secret: %w", err) } return secretName, nil @@ -151,7 +152,7 @@ func (m *Manager) ReadCredentials(ctx context.Context, secretName string, creds return fmt.Errorf("%w: path=%s", credentials.ErrNotFound, secretName) } - return fmt.Errorf("failed to get secret: %w", err) + return errorsx.Errorf("failed to get secret: %w", err) } return json.Unmarshal([]byte(*resp.Value), creds) @@ -161,7 +162,7 @@ func (m *Manager) ReadCredentials(ctx context.Context, secretName string, creds func (m *Manager) DeleteCredentials(ctx context.Context, secretName string) error { _, err := m.client.DeleteSecret(ctx, secretName, nil) if err != nil { - return fmt.Errorf("failed to delete secret: %w", err) + return errorsx.Errorf("failed to delete secret: %w", err) } return nil @@ -174,11 +175,11 @@ func ValidateWriterClient(m *Manager, pathPrefix string) error { ctx := context.Background() if _, err := m.client.SetSecret(ctx, secretName, azsecrets.SetSecretParameters{Value: strPtr("")}, nil); err != nil { - return fmt.Errorf("failed to set secret: %w", err) + return errorsx.Errorf("failed to set secret: %w", err) } if _, err := m.client.DeleteSecret(ctx, secretName, nil); err != nil { - return fmt.Errorf("failed to delete secret: %w", err) + return errorsx.Errorf("failed to delete secret: %w", err) } return nil @@ -201,8 +202,8 @@ func ValidateReaderClient(m *Manager, pathPrefix string) error { return nil } - return fmt.Errorf("failed to get secret: %w", err) + return errorsx.Errorf("failed to get secret: %w", err) } - return errors.New("expected error") + return errorsx.New("expected error") } diff --git a/pkg/credentials/gcp/secretmanager.go b/pkg/credentials/gcp/secretmanager.go index b0c9cc784..1a22fa0d2 100644 --- a/pkg/credentials/gcp/secretmanager.go +++ b/pkg/credentials/gcp/secretmanager.go @@ -18,7 +18,6 @@ package gcp import ( "context" "encoding/json" - "errors" "fmt" "io" "strings" @@ -30,6 +29,7 @@ import ( "google.golang.org/grpc/status" "github.com/chainloop-dev/chainloop/pkg/credentials" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/chainloop-dev/chainloop/pkg/servicelogger" "github.com/docker/distribution/uuid" "github.com/go-kratos/kratos/v2/log" @@ -59,7 +59,7 @@ type NewManagerOpts struct { func NewManager(opts *NewManagerOpts) (*Manager, error) { if opts.ProjectID == "" || opts.ServiceAccountKey == "" { - return nil, errors.New("projectID and serviceAccountKey are required") + return nil, errorsx.New("projectID and serviceAccountKey are required") } l := opts.Logger @@ -72,7 +72,7 @@ func NewManager(opts *NewManagerOpts) (*Manager, error) { cli, err := secretmanager.NewClient(context.TODO(), option.WithCredentialsFile(opts.ServiceAccountKey)) if err != nil { - return nil, fmt.Errorf("error while creating the client: %w", err) + return nil, errorsx.Errorf("error while creating the client: %w", err) } logger.Infow("msg", "created GCP connection", "projectID", opts.ProjectID, "role", opts.Role, "prefix", opts.SecretPrefix) @@ -90,7 +90,7 @@ func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any, // store creds in key-value pair c, err := json.Marshal(creds) if err != nil { - return "", fmt.Errorf("marshaling credentials to be stored: %w", err) + return "", errorsx.Errorf("marshaling credentials to be stored: %w", err) } o := credentials.ApplySaveOptions(opts...) @@ -124,18 +124,18 @@ func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any, return secretID, nil } if !isNotFoundErr(addErr) { - return "", fmt.Errorf("adding secret version in GCP: %w", addErr) + return "", errorsx.Errorf("adding secret version in GCP: %w", addErr) } // Secret container doesn't exist yet — create it and fall through to AddSecretVersion below. if _, err = m.client.CreateSecret(ctx, secretReq); err != nil { - return "", fmt.Errorf("creating secret in GCP: %w", err) + return "", errorsx.Errorf("creating secret in GCP: %w", err) } m.logger.Infow("msg", "created new secret", "secretID", secretID) } else { // New secret: create the container and use the canonical name from the API response. secret, createErr := m.client.CreateSecret(ctx, secretReq) if createErr != nil { - return "", fmt.Errorf("creating secret in GCP: %w", createErr) + return "", errorsx.Errorf("creating secret in GCP: %w", createErr) } parent = secret.Name m.logger.Infow("msg", "created new secret", "secretID", secretID) @@ -147,7 +147,7 @@ func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any, Payload: &secretmanagerpb.SecretPayload{Data: c}, }) if err != nil { - return "", fmt.Errorf("creating secret version in GCP: %w", err) + return "", errorsx.Errorf("creating secret version in GCP: %w", err) } m.logger.Infow("msg", "added new secret version", "secretID", secretID, "versionID", v.Name) @@ -181,5 +181,5 @@ func (m *Manager) DeleteCredentials(ctx context.Context, secretID string) error } m.logger.Infow("msg", "deleting secret", "secretID", secretID) - return m.client.DeleteSecret(ctx, &deleteRequest) + return errorsx.WithStack(m.client.DeleteSecret(ctx, &deleteRequest)) } diff --git a/pkg/credentials/vault/keyval.go b/pkg/credentials/vault/keyval.go index 6321f4940..7b820780e 100644 --- a/pkg/credentials/vault/keyval.go +++ b/pkg/credentials/vault/keyval.go @@ -26,6 +26,7 @@ import ( "time" "github.com/chainloop-dev/chainloop/pkg/credentials" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/chainloop-dev/chainloop/pkg/servicelogger" "github.com/docker/distribution/uuid" "github.com/go-kratos/kratos/v2/log" @@ -60,7 +61,7 @@ const healthCheckNonExisting = "chainloop-non-existing" // SecretPrefix is used to namespace secrets in the KVv2 engine during write operations. func NewManager(opts *NewManagerOpts) (*Manager, error) { if opts.AuthToken == "" || opts.Address == "" { - return nil, errors.New("auth token and instance address are required") + return nil, errorsx.New("auth token and instance address are required") } config := vault.DefaultConfig() @@ -69,7 +70,7 @@ func NewManager(opts *NewManagerOpts) (*Manager, error) { client, err := vault.NewClient(config) if err != nil { - return nil, err + return nil, errorsx.Errorf("creating Vault client: %w", err) } client.SetToken(opts.AuthToken) @@ -108,11 +109,11 @@ func validateWriterClient(kv *vault.KVv2, pathPrefix string) error { ctx := context.Background() keyPath := strings.Join([]string{pathPrefix, healthCheckSecret}, "/") if _, err := kv.Put(ctx, keyPath, nil); err != nil { - return err + return errorsx.Errorf("writing health check secret: %w", err) } if err := kv.DeleteMetadata(ctx, keyPath); err != nil { - return fmt.Errorf("deleting health check secret: %w", err) + return errorsx.Errorf("deleting health check secret: %w", err) } return nil @@ -130,7 +131,7 @@ func validateReaderClient(kv *vault.KVv2, pathPrefix string) error { return nil } - return err + return errorsx.Errorf("reading health check secret: %w", err) } return nil @@ -139,7 +140,7 @@ func validateReaderClient(kv *vault.KVv2, pathPrefix string) error { func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any, opts ...credentials.SaveOption) (string, error) { credsM, err := structToMap(creds) if err != nil { - return "", fmt.Errorf("converting struct to map: %w", err) + return "", errorsx.Errorf("converting struct to map: %w", err) } o := credentials.ApplySaveOptions(opts...) @@ -151,7 +152,7 @@ func (m *Manager) SaveCredentials(ctx context.Context, orgID string, creds any, _, err = m.client.Put(ctx, secretName, credsM) if err != nil { - return "", fmt.Errorf("storing secret in Vault: %w", err) + return "", errorsx.Errorf("storing secret in Vault: %w", err) } return secretName, nil @@ -166,11 +167,11 @@ func (m *Manager) ReadCredentials(ctx context.Context, secretID string, creds an return fmt.Errorf("%w: path=%s", credentials.ErrNotFound, secretID) } - return fmt.Errorf("reading secret from Vault: %w", err) + return errorsx.Errorf("reading secret from Vault: %w", err) } if err := mapToStruct(s.Data, creds); err != nil { - return fmt.Errorf("converting secret to struct: %w", err) + return errorsx.Errorf("converting secret to struct: %w", err) } return nil @@ -178,7 +179,7 @@ func (m *Manager) ReadCredentials(ctx context.Context, secretID string, creds an func (m *Manager) DeleteCredentials(ctx context.Context, secretID string) error { m.logger.Infow("msg", "deleting credentials", "path", secretID) - return m.client.DeleteMetadata(ctx, secretID) + return errorsx.WithStack(m.client.DeleteMetadata(ctx, secretID)) } // convert from struct to map[string]interface{} diff --git a/pkg/grpcconn/grpcconn.go b/pkg/grpcconn/grpcconn.go index e548e5319..4b9e55bbf 100644 --- a/pkg/grpcconn/grpcconn.go +++ b/pkg/grpcconn/grpcconn.go @@ -147,8 +147,10 @@ func New(uri, authToken string, opt ...Option) (*grpc.ClientConn, error) { } var opts []grpc.DialOption - unaryInterceptors := []grpc.UnaryClientInterceptor{} - streamInterceptors := []grpc.StreamClientInterceptor{} + // The stack interceptors come first, so they are outermost: inner + // interceptors such as retry see the errors as gRPC returns them. + unaryInterceptors := []grpc.UnaryClientInterceptor{StackUnaryClientInterceptor()} + streamInterceptors := []grpc.StreamClientInterceptor{StackStreamClientInterceptor()} if authToken != "" || optionalArgs.tokenProvider != nil { grpcCreds := newTokenAuth(authToken, optionalArgs.insecure, optionalArgs.orgName) @@ -165,12 +167,10 @@ func New(uri, authToken string, opt ...Option) (*grpc.ClientConn, error) { unaryInterceptors = append(unaryInterceptors, optionalArgs.unaryInterceptors...) streamInterceptors = append(streamInterceptors, optionalArgs.streamInterceptors...) - if len(unaryInterceptors) > 0 { - opts = append(opts, grpc.WithChainUnaryInterceptor(unaryInterceptors...)) - } - if len(streamInterceptors) > 0 { - opts = append(opts, grpc.WithChainStreamInterceptor(streamInterceptors...)) - } + opts = append(opts, + grpc.WithChainUnaryInterceptor(unaryInterceptors...), + grpc.WithChainStreamInterceptor(streamInterceptors...), + ) // Currently we only support system tls certs var tlsDialOption grpc.DialOption diff --git a/pkg/grpcconn/stack.go b/pkg/grpcconn/stack.go new file mode 100644 index 000000000..9dd3573bb --- /dev/null +++ b/pkg/grpcconn/stack.go @@ -0,0 +1,73 @@ +// +// Copyright 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. +// You may obtain a copy of the License at +// +// http://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 grpcconn + +import ( + "context" + "io" + + "github.com/chainloop-dev/chainloop/pkg/errorsx" + "google.golang.org/grpc" +) + +// StackUnaryClientInterceptor records the stack of the caller on each error of +// a unary call, so Sentry groups a failed call by the code that made it. +// The error keeps its message and its gRPC status. +func StackUnaryClientInterceptor() grpc.UnaryClientInterceptor { + return func(ctx context.Context, method string, req, reply any, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { + return errorsx.WithStack(invoker(ctx, method, req, reply, cc, opts...)) + } +} + +// StackStreamClientInterceptor records the stack of the caller on each error of +// a streaming call: on stream creation, SendMsg, RecvMsg and CloseSend. +// It does not wrap io.EOF, because callers compare it with ==. +func StackStreamClientInterceptor() grpc.StreamClientInterceptor { + return func(ctx context.Context, desc *grpc.StreamDesc, cc *grpc.ClientConn, method string, streamer grpc.Streamer, opts ...grpc.CallOption) (grpc.ClientStream, error) { + cs, err := streamer(ctx, desc, cc, method, opts...) + if err != nil { + return nil, errorsx.WithStack(err) + } + + return &stackClientStream{ClientStream: cs}, nil + } +} + +// stackClientStream is a client stream that records the stack of the caller on each error. +type stackClientStream struct { + grpc.ClientStream +} + +func (s *stackClientStream) SendMsg(m any) error { + return streamErr(s.ClientStream.SendMsg(m)) +} + +func (s *stackClientStream) RecvMsg(m any) error { + return streamErr(s.ClientStream.RecvMsg(m)) +} + +func (s *stackClientStream) CloseSend() error { + return streamErr(s.ClientStream.CloseSend()) +} + +// streamErr adds a stack to err, except to io.EOF, which ends a stream normally. +func streamErr(err error) error { + if err == io.EOF { //nolint:errorlint // gRPC returns io.EOF unwrapped and callers compare it with ==. + return err + } + + return errorsx.WithStack(err) +} diff --git a/pkg/grpcconn/stack_test.go b/pkg/grpcconn/stack_test.go new file mode 100644 index 000000000..528d956bb --- /dev/null +++ b/pkg/grpcconn/stack_test.go @@ -0,0 +1,163 @@ +// +// Copyright 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. +// You may obtain a copy of the License at +// +// http://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 grpcconn + +import ( + "context" + "errors" + "io" + "testing" + + "github.com/chainloop-dev/chainloop/pkg/errorsx" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +func TestStackUnaryClientInterceptor(t *testing.T) { + statusErr := status.Error(codes.NotFound, "missing") + + testCases := []struct { + name string + invokeErr error + wantStack bool + }{ + {name: "nil stays nil", invokeErr: nil}, + {name: "status error gets a stack", invokeErr: statusErr, wantStack: true}, + {name: "plain error gets a stack", invokeErr: errors.New("boom"), wantStack: true}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + invoker := func(context.Context, string, any, any, *grpc.ClientConn, ...grpc.CallOption) error { + return tc.invokeErr + } + + err := StackUnaryClientInterceptor()(context.Background(), "/svc/Method", nil, nil, nil, invoker) + if tc.invokeErr == nil { + assert.NoError(t, err) + return + } + + require.ErrorIs(t, err, tc.invokeErr) + assert.Equal(t, tc.invokeErr.Error(), err.Error()) + assert.Equal(t, tc.wantStack, errorsx.HasStack(err)) + assert.Equal(t, status.Code(tc.invokeErr), status.Code(err)) + }) + } +} + +// fakeClientStream returns the configured errors from its stream methods. +type fakeClientStream struct { + grpc.ClientStream + sendErr, recvErr, closeErr error +} + +func (s *fakeClientStream) SendMsg(any) error { return s.sendErr } +func (s *fakeClientStream) RecvMsg(any) error { return s.recvErr } +func (s *fakeClientStream) CloseSend() error { return s.closeErr } + +func TestStackStreamClientInterceptor(t *testing.T) { + statusErr := status.Error(codes.Unavailable, "down") + + t.Run("stream creation error gets a stack", func(t *testing.T) { + streamer := func(context.Context, *grpc.StreamDesc, *grpc.ClientConn, string, ...grpc.CallOption) (grpc.ClientStream, error) { + return nil, statusErr + } + + cs, err := StackStreamClientInterceptor()(context.Background(), &grpc.StreamDesc{}, nil, "/svc/Stream", streamer) + assert.Nil(t, cs) + require.ErrorIs(t, err, statusErr) + assert.True(t, errorsx.HasStack(err)) + assert.Equal(t, codes.Unavailable, status.Code(err)) + }) + + testCases := []struct { + name string + stream *fakeClientStream + call func(grpc.ClientStream) error + wantErr error + wantStack bool + }{ + { + name: "SendMsg error gets a stack", + stream: &fakeClientStream{sendErr: statusErr}, + call: func(cs grpc.ClientStream) error { return cs.SendMsg(nil) }, + wantErr: statusErr, + wantStack: true, + }, + { + name: "RecvMsg error gets a stack", + stream: &fakeClientStream{recvErr: statusErr}, + call: func(cs grpc.ClientStream) error { return cs.RecvMsg(nil) }, + wantErr: statusErr, + wantStack: true, + }, + { + name: "CloseSend error gets a stack", + stream: &fakeClientStream{closeErr: statusErr}, + call: func(cs grpc.ClientStream) error { return cs.CloseSend() }, + wantErr: statusErr, + wantStack: true, + }, + { + name: "RecvMsg io.EOF stays io.EOF", + stream: &fakeClientStream{recvErr: io.EOF}, + call: func(cs grpc.ClientStream) error { return cs.RecvMsg(nil) }, + wantErr: io.EOF, + }, + { + name: "SendMsg io.EOF stays io.EOF", + stream: &fakeClientStream{sendErr: io.EOF}, + call: func(cs grpc.ClientStream) error { return cs.SendMsg(nil) }, + wantErr: io.EOF, + }, + { + name: "nil stays nil", + stream: &fakeClientStream{}, + call: func(cs grpc.ClientStream) error { return cs.RecvMsg(nil) }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + streamer := func(context.Context, *grpc.StreamDesc, *grpc.ClientConn, string, ...grpc.CallOption) (grpc.ClientStream, error) { + return tc.stream, nil + } + + cs, err := StackStreamClientInterceptor()(context.Background(), &grpc.StreamDesc{}, nil, "/svc/Stream", streamer) + require.NoError(t, err) + + err = tc.call(cs) + if tc.wantErr == nil { + assert.NoError(t, err) + return + } + + if !tc.wantStack { + // Callers compare io.EOF with ==, so it must not be wrapped. + assert.Same(t, tc.wantErr, err) //nolint:testifylint + return + } + + require.ErrorIs(t, err, tc.wantErr) + assert.True(t, errorsx.HasStack(err)) + assert.Equal(t, status.Code(tc.wantErr), status.Code(err)) + }) + } +} diff --git a/pkg/natsconn/natsconn.go b/pkg/natsconn/natsconn.go index 8ba7aecbb..c96ec9d99 100644 --- a/pkg/natsconn/natsconn.go +++ b/pkg/natsconn/natsconn.go @@ -17,10 +17,10 @@ package natsconn import ( "context" - "fmt" "sync" "time" + "github.com/chainloop-dev/chainloop/pkg/errorsx" "github.com/go-kratos/kratos/v2/log" "github.com/nats-io/nats.go" ) @@ -78,7 +78,7 @@ func New(cfg *Config, logger log.Logger) (*ReloadableConnection, func(), error) nc, err := nats.Connect(cfg.URI, opts...) if err != nil { - return nil, noop, fmt.Errorf("connecting to NATS: %w", err) + return nil, noop, errorsx.Errorf("connecting to NATS: %w", err) } rc.Conn = nc diff --git a/pkg/policies/fetch_remote_test.go b/pkg/policies/fetch_remote_test.go new file mode 100644 index 000000000..629a43cd4 --- /dev/null +++ b/pkg/policies/fetch_remote_test.go @@ -0,0 +1,73 @@ +// +// Copyright 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. +// You may obtain a copy of the License at +// +// http://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 policies + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/chainloop-dev/chainloop/pkg/errorsx" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFetchRemoteErrorsRecordStack(t *testing.T) { + notFound := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + })) + defer notFound.Close() + + closed := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + // Close the server so that the request fails in the transport. + closed.Close() + + testCases := []struct { + name string + url string + wantStatus int + wantMsg string + }{ + { + name: "non-2xx status returns a typed error", + url: notFound.URL, + wantStatus: http.StatusNotFound, + wantMsg: "requesting remote policy: server returned HTTP 404", + }, + { + name: "transport failure", + url: closed.URL, + wantMsg: "requesting remote policy: ", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + _, err := fetchRemote(nil, tc.url) + require.Error(t, err) + assert.Contains(t, err.Error(), tc.wantMsg) + assert.True(t, errorsx.HasStack(err)) + + var statusErr *RemoteStatusError + if tc.wantStatus == 0 { + assert.NotErrorAs(t, err, &statusErr) + return + } + require.ErrorAs(t, err, &statusErr) + assert.Equal(t, tc.wantStatus, statusErr.StatusCode) + }) + } +} diff --git a/pkg/policies/loader.go b/pkg/policies/loader.go index 1e7e104d4..957026669 100644 --- a/pkg/policies/loader.go +++ b/pkg/policies/loader.go @@ -32,6 +32,7 @@ import ( v1 "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/unmarshal" "github.com/chainloop-dev/chainloop/pkg/cache" + "github.com/chainloop-dev/chainloop/pkg/errorsx" crv1 "github.com/google/go-containerregistry/pkg/v1" "golang.org/x/sync/singleflight" "google.golang.org/protobuf/encoding/protojson" @@ -171,22 +172,34 @@ func fetchRemote(client *http.Client, ref string) ([]byte, error) { if errors.As(err, &urlErr) { err = urlErr.Err } - return nil, fmt.Errorf("requesting remote policy: %w", err) + return nil, errorsx.Errorf("requesting remote policy: %w", err) } defer resp.Body.Close() if resp.StatusCode < 200 || resp.StatusCode >= 300 { - return nil, fmt.Errorf("requesting remote policy: server returned HTTP %d", resp.StatusCode) + return nil, errorsx.Errorf("requesting remote policy: %w", &RemoteStatusError{StatusCode: resp.StatusCode}) } raw, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("reading remote policy: %w", err) + return nil, errorsx.Errorf("reading remote policy: %w", err) } return raw, nil } +// RemoteStatusError is returned when the server of a remote policy or policy +// group answers with a status other than 2xx. It has its own type so that +// Sentry does not group it with transport failures. +type RemoteStatusError struct { + // StatusCode is the HTTP status code of the response. + StatusCode int +} + +func (e *RemoteStatusError) Error() string { + return fmt.Sprintf("server returned HTTP %d", e.StatusCode) +} + // ChainloopLoader loads policies referenced with chainloop://provider/name URLs type ChainloopLoader struct { Client pb.AttestationServiceClient From eaa15cecd7112239acc40d21419473e49800af52 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Sun, 1 Nov 2026 00:02:07 +0100 Subject: [PATCH 2/3] fix: address review on error stack boundaries Wrap the gRPC stream Header error, forward CloseIdleConnections through the plugin stack transport, record the Dependency-Track status error stack at each request, close policy provider response bodies, and test the OIDC error stacks. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: b3cc191e-bd8c-4933-beb5-9a5ae1f2331c --- .../internal/oidcauthenticator/auth_test.go | 78 +++++++++++++++++++ .../pkg/policies/policyprovider.go | 4 +- .../core/dependency-track/v1/client/sbom.go | 11 +-- app/controlplane/plugins/sdk/v1/httpclient.go | 8 ++ pkg/grpcconn/stack.go | 8 +- pkg/grpcconn/stack_test.go | 20 ++++- 6 files changed, 114 insertions(+), 15 deletions(-) create mode 100644 app/controlplane/internal/oidcauthenticator/auth_test.go diff --git a/app/controlplane/internal/oidcauthenticator/auth_test.go b/app/controlplane/internal/oidcauthenticator/auth_test.go new file mode 100644 index 000000000..d75f8eda2 --- /dev/null +++ b/app/controlplane/internal/oidcauthenticator/auth_test.go @@ -0,0 +1,78 @@ +// +// Copyright 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. +// You may obtain a copy of the License at +// +// http://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 authenticator + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/chainloop-dev/chainloop/pkg/errorsx" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/oauth2" +) + +func TestNewOIDCDiscoveryFailureRecordsStack(t *testing.T) { + srv := httptest.NewServer(http.NotFoundHandler()) + t.Cleanup(srv.Close) + + _, err := NewOIDC(srv.URL, "client", "secret", "http://localhost/callback") + require.Error(t, err) + assert.True(t, errorsx.HasStack(err)) +} + +func TestVerifyIDTokenErrorsRecordStack(t *testing.T) { + testCases := []struct { + name string + token *oauth2.Token + }{ + {name: "missing id_token", token: &oauth2.Token{}}, + {name: "invalid id_token", token: (&oauth2.Token{}).WithExtra(map[string]any{"id_token": "not-a-jwt"})}, + } + + srv := newDiscoveryServer(t) + a, err := NewOIDC(srv.URL, "client", "secret", "http://localhost/callback") + require.NoError(t, err) + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + _, err := a.VerifyIDToken(context.Background(), tc.token) + require.Error(t, err) + assert.True(t, errorsx.HasStack(err)) + }) + } +} + +// newDiscoveryServer serves a minimal OIDC discovery document for its own URL. +func newDiscoveryServer(t *testing.T) *httptest.Server { + t.Helper() + + var srv *httptest.Server + srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/.well-known/openid-configuration" { + http.NotFound(w, r) + return + } + + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"issuer":"` + srv.URL + `","authorization_endpoint":"` + srv.URL + `/auth","token_endpoint":"` + srv.URL + `/token","jwks_uri":"` + srv.URL + `/keys"}`)) + })) + t.Cleanup(srv.Close) + + return srv +} diff --git a/app/controlplane/pkg/policies/policyprovider.go b/app/controlplane/pkg/policies/policyprovider.go index 88918adb0..6dbcdda98 100644 --- a/app/controlplane/pkg/policies/policyprovider.go +++ b/app/controlplane/pkg/policies/policyprovider.go @@ -214,6 +214,7 @@ func (p *PolicyProvider) ValidateAttachment(att *schemaapi.PolicyAttachment, aut if err != nil { return err } + defer resp.Body.Close() if resp.StatusCode != http.StatusOK { switch resp.StatusCode { @@ -232,8 +233,6 @@ func (p *PolicyProvider) ValidateAttachment(att *schemaapi.PolicyAttachment, aut return errorsx.Errorf("error reading validation response: %w", err) } - defer resp.Body.Close() - // unmarshall response var response ValidateResponse if err := json.Unmarshal(resBytes, &response); err != nil { @@ -313,6 +312,7 @@ func (p *PolicyProvider) queryProvider(url *url.URL, digest, orgName string, aut if err != nil { return "", "", err } + defer resp.Body.Close() if resp.StatusCode != http.StatusOK { switch resp.StatusCode { diff --git a/app/controlplane/plugins/core/dependency-track/v1/client/sbom.go b/app/controlplane/plugins/core/dependency-track/v1/client/sbom.go index 7840f5807..c8a8e0cfc 100644 --- a/app/controlplane/plugins/core/dependency-track/v1/client/sbom.go +++ b/app/controlplane/plugins/core/dependency-track/v1/client/sbom.go @@ -224,7 +224,7 @@ func teamPermissionsRequest(httpClient *http.Client, host *url.URL, apiKey strin // Check the response if res.StatusCode != http.StatusOK { - return nil, newStatusError(res) + return nil, errorsx.WithStack(&StatusError{Status: res.Status, StatusCode: res.StatusCode}) } resBody, err := io.ReadAll(res.Body) @@ -287,7 +287,7 @@ func uploadSBOMRequest(httpClient *http.Client, host *url.URL, apiKey string, va // Check the response if res.StatusCode != http.StatusOK { - return nil, newStatusError(res) + return nil, errorsx.WithStack(&StatusError{Status: res.Status, StatusCode: res.StatusCode}) } resBody, err := io.ReadAll(res.Body) @@ -327,7 +327,7 @@ func projectExists(httpClient *http.Client, host *url.URL, apiKey string, projec case http.StatusNotFound: return false, nil default: - return false, newStatusError(res) + return false, errorsx.WithStack(&StatusError{Status: res.Status, StatusCode: res.StatusCode}) } } @@ -342,8 +342,3 @@ type StatusError struct { } func (e *StatusError) Error() string { return "bad status: " + e.Status } - -// newStatusError returns a StatusError for res with the stack of the caller. -func newStatusError(res *http.Response) error { - return errorsx.WithStack(&StatusError{Status: res.Status, StatusCode: res.StatusCode}) -} diff --git a/app/controlplane/plugins/sdk/v1/httpclient.go b/app/controlplane/plugins/sdk/v1/httpclient.go index 5aad086db..48b474d41 100644 --- a/app/controlplane/plugins/sdk/v1/httpclient.go +++ b/app/controlplane/plugins/sdk/v1/httpclient.go @@ -106,3 +106,11 @@ func (t *stackTransport) RoundTrip(req *http.Request) (*http.Response, error) { return resp, nil } + +// CloseIdleConnections forwards to the base transport, so +// http.Client.CloseIdleConnections still releases pooled connections. +func (t *stackTransport) CloseIdleConnections() { + if c, ok := t.base.(interface{ CloseIdleConnections() }); ok { + c.CloseIdleConnections() + } +} diff --git a/pkg/grpcconn/stack.go b/pkg/grpcconn/stack.go index 9dd3573bb..ba14cd16d 100644 --- a/pkg/grpcconn/stack.go +++ b/pkg/grpcconn/stack.go @@ -21,6 +21,7 @@ import ( "github.com/chainloop-dev/chainloop/pkg/errorsx" "google.golang.org/grpc" + "google.golang.org/grpc/metadata" ) // StackUnaryClientInterceptor records the stack of the caller on each error of @@ -33,7 +34,7 @@ func StackUnaryClientInterceptor() grpc.UnaryClientInterceptor { } // StackStreamClientInterceptor records the stack of the caller on each error of -// a streaming call: on stream creation, SendMsg, RecvMsg and CloseSend. +// a streaming call: on stream creation, Header, SendMsg, RecvMsg and CloseSend. // It does not wrap io.EOF, because callers compare it with ==. func StackStreamClientInterceptor() grpc.StreamClientInterceptor { return func(ctx context.Context, desc *grpc.StreamDesc, cc *grpc.ClientConn, method string, streamer grpc.Streamer, opts ...grpc.CallOption) (grpc.ClientStream, error) { @@ -63,6 +64,11 @@ func (s *stackClientStream) CloseSend() error { return streamErr(s.ClientStream.CloseSend()) } +func (s *stackClientStream) Header() (metadata.MD, error) { + md, err := s.ClientStream.Header() + return md, streamErr(err) +} + // streamErr adds a stack to err, except to io.EOF, which ends a stream normally. func streamErr(err error) error { if err == io.EOF { //nolint:errorlint // gRPC returns io.EOF unwrapped and callers compare it with ==. diff --git a/pkg/grpcconn/stack_test.go b/pkg/grpcconn/stack_test.go index 528d956bb..d152f6ca8 100644 --- a/pkg/grpcconn/stack_test.go +++ b/pkg/grpcconn/stack_test.go @@ -26,6 +26,7 @@ import ( "github.com/stretchr/testify/require" "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/metadata" "google.golang.org/grpc/status" ) @@ -65,12 +66,13 @@ func TestStackUnaryClientInterceptor(t *testing.T) { // fakeClientStream returns the configured errors from its stream methods. type fakeClientStream struct { grpc.ClientStream - sendErr, recvErr, closeErr error + sendErr, recvErr, closeErr, headerErr error } -func (s *fakeClientStream) SendMsg(any) error { return s.sendErr } -func (s *fakeClientStream) RecvMsg(any) error { return s.recvErr } -func (s *fakeClientStream) CloseSend() error { return s.closeErr } +func (s *fakeClientStream) SendMsg(any) error { return s.sendErr } +func (s *fakeClientStream) RecvMsg(any) error { return s.recvErr } +func (s *fakeClientStream) CloseSend() error { return s.closeErr } +func (s *fakeClientStream) Header() (metadata.MD, error) { return nil, s.headerErr } func TestStackStreamClientInterceptor(t *testing.T) { statusErr := status.Error(codes.Unavailable, "down") @@ -115,6 +117,16 @@ func TestStackStreamClientInterceptor(t *testing.T) { wantErr: statusErr, wantStack: true, }, + { + name: "Header error gets a stack", + stream: &fakeClientStream{headerErr: statusErr}, + call: func(cs grpc.ClientStream) error { + _, err := cs.Header() + return err + }, + wantErr: statusErr, + wantStack: true, + }, { name: "RecvMsg io.EOF stays io.EOF", stream: &fakeClientStream{recvErr: io.EOF}, From acffde116e87fd6272bebb18c8fb50980e28d44e Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Sun, 1 Nov 2026 00:02:39 +0100 Subject: [PATCH 3/3] fix: drop unused stack on OIDC code exchange error The token exchange error ends in a 401 that is never captured. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino --- app/controlplane/internal/service/auth.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/controlplane/internal/service/auth.go b/app/controlplane/internal/service/auth.go index fa07f8777..810c9b274 100644 --- a/app/controlplane/internal/service/auth.go +++ b/app/controlplane/internal/service/auth.go @@ -37,7 +37,6 @@ import ( "github.com/chainloop-dev/chainloop/app/controlplane/pkg/jwt" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/jwt/user" "github.com/chainloop-dev/chainloop/internal/oauth" - "github.com/chainloop-dev/chainloop/pkg/errorsx" sl "github.com/chainloop-dev/chainloop/pkg/servicelogger" kerrors "github.com/go-kratos/kratos/v2/errors" "github.com/go-kratos/kratos/v2/log" @@ -701,7 +700,7 @@ func extractUserInfoFromToken(ctx context.Context, svc *AuthService, r *http.Req // Exchange the code for a token oauth2Token, err := svc.authenticator.Exchange(ctx, code) if err != nil { - return nil, newOauthResp(http.StatusUnauthorized, errorsx.WithStack(err), false) + return nil, newOauthResp(http.StatusUnauthorized, err, false) } // It's a valid Oauth2 token