From f315e32c0c5579159489d57d7199015035591f48 Mon Sep 17 00:00:00 2001 From: Vibhav Bobade Date: Wed, 29 Apr 2026 18:36:10 +0530 Subject: [PATCH] fix(crafter): surface auto-discovery validation errors Stop probing material kinds when auto-detection hits a protovalidate ValidationError. Schema-level failures, such as invalid material names, are not kind mismatches and should be surfaced directly. Fixes: chainloop-dev/chainloop#2394 Assisted-by: pi Signed-off-by: Vibhav Bobade --- pkg/attestation/crafter/crafter.go | 16 +++++++++------- pkg/attestation/crafter/crafter_test.go | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/pkg/attestation/crafter/crafter.go b/pkg/attestation/crafter/crafter.go index 5a2db615b..da53641f4 100644 --- a/pkg/attestation/crafter/crafter.go +++ b/pkg/attestation/crafter/crafter.go @@ -653,31 +653,33 @@ func (c *Crafter) IsMaterialInContract(key string) bool { // AddMaterialContactFreeWithAutoDetectedKind adds a material to the crafting state checking the incoming material matches any of the // supported types in validation order. If the material is not found it will return an error. func (c *Crafter) AddMaterialContactFreeWithAutoDetectedKind(ctx context.Context, attestationID, name, value string, casBackend *casclient.CASBackend, runtimeAnnotations map[string]string, opts ...AddOpt) (*api.Attestation_Material, error) { - var err error for _, kind := range schemaapi.CraftingMaterialInValidationOrder { m, err := c.AddMaterialContractFree(ctx, attestationID, kind.String(), name, value, casBackend, runtimeAnnotations, opts...) if err == nil { - // Successfully added material, return the kind return m, nil } c.Logger.Debug().Err(err).Str("kind", kind.String()).Msg("failed to add material") - // Handle base error for upload and craft errors except the opening file error - // TODO: have an error to detect validation error instead var policyError *policies.PolicyError if errors.Is(err, materials.ErrBaseUploadAndCraft) || errors.As(err, &policyError) { return nil, err } - // This is a final error, we detected the kind if v1.IsAttestationStateErrorConflict(err) { return nil, err } + + // Proto-validation errors (e.g. invalid material name) are schema-level + // failures, not kind mismatches. Stop probing immediately so the real + // error is surfaced to the user instead of being masked by the loop. + var valErr *protovalidate.ValidationError + if errors.As(err, &valErr) { + return nil, err + } } - // Return an error if no material could be added - return nil, fmt.Errorf("failed to auto-discover material kind: %w", err) + return nil, errors.New("failed to auto-discover material kind") } // addMaterials adds the incoming material m to the crafting state diff --git a/pkg/attestation/crafter/crafter_test.go b/pkg/attestation/crafter/crafter_test.go index 1a8feb196..e6432fe7e 100644 --- a/pkg/attestation/crafter/crafter_test.go +++ b/pkg/attestation/crafter/crafter_test.go @@ -672,6 +672,20 @@ func (s *crafterSuite) TestAddMaterialsAutomatic() { } } +func (s *crafterSuite) TestAddMaterialsAutomaticInvalidNameSurfacesValidationError() { + var runner crafter.SupportedRunner = runners.NewGeneric() + uploader := mUploader.NewUploader(s.T()) + + c, err := newInitializedCrafter(s.T(), "testdata/contracts/empty_generic.yaml", &v1.WorkflowMetadata{}, false, "", runner) + require.NoError(s.T(), err) + + m, err := c.AddMaterialContactFreeWithAutoDetectedKind(context.Background(), "random-id", "invalid_name", "./materials/testdata/junit.xml", &casclient.CASBackend{Uploader: uploader}, nil) + require.Error(s.T(), err) + assert.Nil(s.T(), m) + assert.Contains(s.T(), err.Error(), "must contain only lowercase letters, numbers, and hyphens") + assert.NotContains(s.T(), err.Error(), "failed to auto-discover material kind") +} + func loadSchema(path string) (*schemaapi.CraftingSchema, error) { // Extract json formatted data content, err := os.ReadFile(filepath.Clean(path))