Skip to content

Commit 3f550bd

Browse files
committed
fix: surface proper parsing errors for malformed v2 contracts
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
1 parent b200376 commit 3f550bd

2 files changed

Lines changed: 198 additions & 8 deletions

File tree

app/controlplane/internal/service/workflowcontract.go

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package service
1717

1818
import (
1919
"context"
20+
"fmt"
2021

2122
pb "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1"
2223
schemav1 "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1"
@@ -465,7 +466,7 @@ func validateAndExtractMetadata(rawContract []byte, explicitName, explicitDesc s
465466
return name, description, nil
466467
}
467468

468-
// extractNameFromMetadata attempts to extract the name from metadata.name in v2 contracts
469+
// extractMetadata attempts to extract the name from metadata.name in v2 contracts
469470
func extractMetadata(rawContract []byte) (*schemav1.Metadata, error) {
470471
if len(rawContract) == 0 {
471472
return nil, nil
@@ -474,17 +475,33 @@ func extractMetadata(rawContract []byte) (*schemav1.Metadata, error) {
474475
// Identify the format
475476
format, err := unmarshal.IdentifyFormat(rawContract)
476477
if err != nil {
477-
return nil, errors.BadRequest("invalid", "failed to identify contract format")
478+
return nil, nil
479+
}
480+
481+
// Do a lenient parse (no validation) to detect v2 markers.
482+
v2Probe := &schemav1.CraftingSchemaV2{}
483+
lenientErr := unmarshal.FromRaw(rawContract, format, v2Probe, false)
484+
485+
isV2 := v2Probe.GetApiVersion() == "chainloop.dev/v1" && v2Probe.GetKind() == "Contract"
486+
if !isV2 {
487+
// Not a v2 contract
488+
return nil, nil
478489
}
479490

480-
// Try parsing as v2 Contract
491+
// It's a v2 contract. If the lenient parse already failed, surface that error
492+
if lenientErr != nil {
493+
return nil, errors.BadRequest("invalid", fmt.Sprintf("invalid contract: %s", lenientErr))
494+
}
495+
496+
// Lenient parse succeeded, validate
481497
v2Contract := &schemav1.CraftingSchemaV2{}
482-
if err := unmarshal.FromRaw(rawContract, format, v2Contract, true); err == nil {
483-
if v2Contract.GetMetadata() != nil {
484-
return v2Contract.GetMetadata(), nil
485-
}
498+
if err := unmarshal.FromRaw(rawContract, format, v2Contract, true); err != nil {
499+
return nil, errors.BadRequest("invalid", fmt.Sprintf("invalid contract: %s", err))
500+
}
501+
502+
if v2Contract.GetMetadata() != nil {
503+
return v2Contract.GetMetadata(), nil
486504
}
487505

488-
// If v2 parsing failed or no metadata, return nothing
489506
return nil, nil
490507
}
Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
//
2+
// Copyright 2026 The Chainloop Authors.
3+
//
4+
// Licensed under the Apache License, Version 2.0 (the "License");
5+
// you may not use this file except in compliance with the License.
6+
// You may obtain a copy of the License at
7+
//
8+
// http://www.apache.org/licenses/LICENSE-2.0
9+
//
10+
// Unless required by applicable law or agreed to in writing, software
11+
// distributed under the License is distributed on an "AS IS" BASIS,
12+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
// See the License for the specific language governing permissions and
14+
// limitations under the License.
15+
16+
package service
17+
18+
import (
19+
"testing"
20+
21+
"github.com/stretchr/testify/assert"
22+
"github.com/stretchr/testify/require"
23+
)
24+
25+
func TestExtractMetadata(t *testing.T) {
26+
t.Parallel()
27+
28+
tests := []struct {
29+
name string
30+
rawContract []byte
31+
wantName string
32+
wantErr bool
33+
errContains string
34+
}{
35+
{
36+
name: "empty content returns nil",
37+
rawContract: []byte{},
38+
wantName: "",
39+
wantErr: false,
40+
},
41+
{
42+
name: "valid v2 contract returns metadata",
43+
rawContract: []byte(`
44+
apiVersion: chainloop.dev/v1
45+
kind: Contract
46+
metadata:
47+
name: my-contract
48+
description: a test contract
49+
spec:
50+
materials:
51+
- type: ARTIFACT
52+
name: my-artifact
53+
`),
54+
wantName: "my-contract",
55+
wantErr: false,
56+
},
57+
{
58+
name: "v2 contract with structural error returns parsing error",
59+
rawContract: []byte(`
60+
apiVersion: chainloop.dev/v1
61+
kind: Contract
62+
metadata:
63+
name: my-contract
64+
spec:
65+
materials:
66+
ref: vulnerabilities
67+
`),
68+
wantErr: true,
69+
errContains: "invalid contract",
70+
},
71+
{
72+
name: "non-v2 content returns nil",
73+
rawContract: []byte(`
74+
schemaVersion: v1
75+
materials:
76+
- type: ARTIFACT
77+
name: my-artifact
78+
`),
79+
wantName: "",
80+
wantErr: false,
81+
},
82+
{
83+
name: "unparseable content returns nil",
84+
rawContract: []byte("\x00\x01\x02"),
85+
wantName: "",
86+
wantErr: false,
87+
},
88+
}
89+
90+
for _, tc := range tests {
91+
t.Run(tc.name, func(t *testing.T) {
92+
t.Parallel()
93+
94+
metadata, err := extractMetadata(tc.rawContract)
95+
if tc.wantErr {
96+
require.Error(t, err)
97+
assert.Contains(t, err.Error(), tc.errContains)
98+
return
99+
}
100+
101+
require.NoError(t, err)
102+
if tc.wantName == "" {
103+
assert.Nil(t, metadata)
104+
} else {
105+
require.NotNil(t, metadata)
106+
assert.Equal(t, tc.wantName, metadata.GetName())
107+
}
108+
})
109+
}
110+
}
111+
112+
func TestValidateAndExtractMetadata(t *testing.T) {
113+
t.Parallel()
114+
115+
tests := []struct {
116+
name string
117+
rawContract []byte
118+
explicitName string
119+
explicitDesc string
120+
wantName string
121+
wantDesc *string
122+
wantErr bool
123+
errContains string
124+
}{
125+
{
126+
name: "explicit name with no contract",
127+
rawContract: nil,
128+
explicitName: "my-workflow",
129+
wantName: "my-workflow",
130+
wantErr: false,
131+
},
132+
{
133+
name: "no name and no contract returns error",
134+
rawContract: nil,
135+
wantErr: true,
136+
errContains: "name is required",
137+
},
138+
{
139+
name: "v2 contract with structural error surfaces parsing error",
140+
rawContract: []byte(`
141+
apiVersion: chainloop.dev/v1
142+
kind: Contract
143+
metadata:
144+
name: my-contract
145+
spec:
146+
materials:
147+
ref: vulnerabilities
148+
`),
149+
wantErr: true,
150+
errContains: "invalid contract",
151+
},
152+
}
153+
154+
for _, tc := range tests {
155+
t.Run(tc.name, func(t *testing.T) {
156+
t.Parallel()
157+
158+
name, desc, err := validateAndExtractMetadata(tc.rawContract, tc.explicitName, tc.explicitDesc)
159+
if tc.wantErr {
160+
require.Error(t, err)
161+
assert.Contains(t, err.Error(), tc.errContains)
162+
return
163+
}
164+
165+
require.NoError(t, err)
166+
assert.Equal(t, tc.wantName, name)
167+
if tc.wantDesc != nil {
168+
require.NotNil(t, desc)
169+
assert.Equal(t, *tc.wantDesc, *desc)
170+
}
171+
})
172+
}
173+
}

0 commit comments

Comments
 (0)