Skip to content

Commit bbc99ec

Browse files
TristanSpeakEasyTristanSpeakEasy
committed
fix: resolve Arazzo validation bug for JSONPath expressions in request body payloads
1 parent 2816fa5 commit bbc99ec

File tree

12 files changed

+204
-112
lines changed

12 files changed

+204
-112
lines changed

‎arazzo/criterion/condition.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ func (s *Condition) Validate(line, column int, opts ...validation.Option) []erro
8787
})
8888
}
8989

90-
if err := s.Expression.Validate(true); err != nil {
90+
if err := s.Expression.Validate(); err != nil {
9191
errs = append(errs, &validation.Error{
9292
UnderlyingError: validation.NewValueValidationError(err.Error()),
9393
Line: line,

‎arazzo/criterion/criterion.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ func (c *Criterion) Validate(opts ...validation.Option) []error {
211211
}
212212

213213
if c.Context != nil {
214-
if err := c.Context.Validate(true); err != nil {
214+
if err := c.Context.Validate(); err != nil {
215215
errs = append(errs, validation.NewValueError(validation.NewValueValidationError(err.Error()), core, core.Context))
216216
}
217217
}

‎arazzo/parameter.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ func (p *Parameter) Validate(ctx context.Context, opts ...validation.Option) []e
8888
errs = append(errs, validation.NewValueError(validation.NewValueValidationError(err.Error()), core, core.Value))
8989
}
9090
if expression != nil {
91-
if err := expression.Validate(true); err != nil {
91+
if err := expression.Validate(); err != nil {
9292
errs = append(errs, validation.NewValueError(validation.NewValueValidationError(err.Error()), core, core.Value))
9393
}
9494
}

‎arazzo/payloadreplacement.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func (p *PayloadReplacement) Validate(ctx context.Context, opts ...validation.Op
4747
errs = append(errs, validation.NewValueError(validation.NewValueValidationError(err.Error()), core, core.Value))
4848
}
4949
if expression != nil {
50-
if err := expression.Validate(true); err != nil {
50+
if err := expression.Validate(); err != nil {
5151
errs = append(errs, validation.NewValueError(validation.NewValueValidationError(err.Error()), core, core.Value))
5252
}
5353
}

‎arazzo/requestbody.go

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"github.com/speakeasy-api/openapi/internal/interfaces"
1111
"github.com/speakeasy-api/openapi/marshaller"
1212
"github.com/speakeasy-api/openapi/validation"
13-
"gopkg.in/yaml.v3"
1413
)
1514

1615
// RequestBody represents the request body to pass to an operation represented by a step
@@ -41,18 +40,17 @@ func (r *RequestBody) Validate(ctx context.Context, opts ...validation.Option) [
4140
}
4241
}
4342

43+
// Validate payload only if it is a top-level Arazzo runtime expression
44+
// Skip validation for values containing embedded expressions or arbitrary data
4445
if r.Payload != nil {
45-
payloadData, err := yaml.Marshal(r.Payload)
46-
if err != nil {
47-
errs = append(errs, validation.NewValueError(validation.NewValueValidationError(err.Error()), core, core.Payload))
48-
}
49-
50-
expressions := expression.ExtractExpressions(string(payloadData))
51-
for _, expression := range expressions {
52-
if err := expression.Validate(true); err != nil {
53-
errs = append(errs, validation.NewValueError(validation.NewValueValidationError(err.Error()), core, core.Payload))
46+
_, exp, err := expression.GetValueOrExpressionValue(r.Payload)
47+
if err == nil && exp != nil {
48+
// Only validate if the entire payload IS an expression (not just contains expressions)
49+
if err := exp.Validate(); err != nil {
50+
errs = append(errs, validation.NewValueError(validation.NewValueValidationError("payload expression is not valid: %s", err), core, core.Payload))
5451
}
5552
}
53+
// If exp is nil, the payload is a value (not an expression) - no validation needed
5654
}
5755

5856
for _, replacement := range r.Replacements {

‎arazzo/requestbody_test.go

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
package arazzo_test
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/speakeasy-api/openapi/arazzo"
8+
"github.com/speakeasy-api/openapi/arazzo/core"
9+
"github.com/speakeasy-api/openapi/marshaller"
10+
"github.com/speakeasy-api/openapi/pointer"
11+
"github.com/stretchr/testify/assert"
12+
"gopkg.in/yaml.v3"
13+
)
14+
15+
func TestRequestBody_Validate_JSONPathInPayload_Success(t *testing.T) {
16+
// This test reproduces the bug where JSONPath-like expressions in payload
17+
// are incorrectly validated as Arazzo runtime expressions
18+
ctx := context.Background()
19+
20+
// Create a payload that contains JSONPath-like expressions ($.Id, $.time, etc.)
21+
// but these should NOT be validated as Arazzo expressions since they're just payload data
22+
payloadNode := &yaml.Node{
23+
Kind: yaml.MappingNode,
24+
Content: []*yaml.Node{
25+
{Kind: yaml.ScalarNode, Value: "dataSource"},
26+
{Kind: yaml.ScalarNode, Value: "/query?query=select * from Account"},
27+
{Kind: yaml.ScalarNode, Value: "keyBy"},
28+
{Kind: yaml.SequenceNode, Content: []*yaml.Node{
29+
{Kind: yaml.ScalarNode, Value: "$.Id"},
30+
}},
31+
{Kind: yaml.ScalarNode, Value: "requiredData"},
32+
{Kind: yaml.MappingNode, Content: []*yaml.Node{
33+
{Kind: yaml.ScalarNode, Value: "Currentbal"},
34+
{Kind: yaml.ScalarNode, Value: "$.CurrentBalance"},
35+
{Kind: yaml.ScalarNode, Value: "SubAcc"},
36+
{Kind: yaml.ScalarNode, Value: "$.SubAccount"},
37+
{Kind: yaml.ScalarNode, Value: "id"},
38+
{Kind: yaml.ScalarNode, Value: "$.Id"},
39+
}},
40+
{Kind: yaml.ScalarNode, Value: "sourceModifiedDate"},
41+
{Kind: yaml.SequenceNode, Content: []*yaml.Node{
42+
{Kind: yaml.ScalarNode, Value: "$.time"},
43+
}},
44+
},
45+
}
46+
47+
requestBody := &arazzo.RequestBody{
48+
ContentType: pointer.From("application/json"),
49+
Payload: payloadNode,
50+
Model: marshaller.Model[core.RequestBody]{
51+
Valid: true,
52+
},
53+
}
54+
55+
// This should NOT return validation errors for the JSONPath expressions in the payload
56+
errs := requestBody.Validate(ctx)
57+
58+
// The bug causes validation errors like:
59+
// "expression is not valid, must begin with one of [url, method, statusCode, request, response, inputs, outputs, steps, workflows, sourceDescriptions, components]: $.time"
60+
assert.Empty(t, errs, "JSONPath-like expressions in payload should not be validated as Arazzo expressions")
61+
}
62+
63+
func TestRequestBody_Validate_AnyPayloadData_Success(t *testing.T) {
64+
// This test ensures that ANY data in payloads is allowed, including invalid expressions
65+
// since payloads are arbitrary user data and should not be validated as Arazzo expressions
66+
ctx := context.Background()
67+
68+
// Create a payload with various expression-like data that should all be ignored
69+
payloadNode := &yaml.Node{
70+
Kind: yaml.MappingNode,
71+
Content: []*yaml.Node{
72+
{Kind: yaml.ScalarNode, Value: "invalidExpression"},
73+
{Kind: yaml.ScalarNode, Value: "$invalidExpression"}, // Would be invalid if validated
74+
{Kind: yaml.ScalarNode, Value: "jsonPath"},
75+
{Kind: yaml.ScalarNode, Value: "$.field.subfield"}, // JSONPath expression
76+
{Kind: yaml.ScalarNode, Value: "template"},
77+
{Kind: yaml.ScalarNode, Value: "${variable}"}, // Template-like expression
78+
},
79+
}
80+
81+
requestBody := &arazzo.RequestBody{
82+
ContentType: pointer.From("application/json"),
83+
Payload: payloadNode,
84+
Model: marshaller.Model[core.RequestBody]{
85+
Valid: true,
86+
},
87+
}
88+
89+
errs := requestBody.Validate(ctx)
90+
91+
// All payload data should be allowed without validation errors
92+
assert.Empty(t, errs, "Payload data should not be validated as Arazzo expressions")
93+
}
94+
95+
func TestRequestBody_Validate_TopLevelExpression_ValidatesCorrectly(t *testing.T) {
96+
// Test that top-level Arazzo expressions are properly validated
97+
98+
// Test valid top-level expression
99+
validRequestBody := &arazzo.RequestBody{}
100+
101+
validPayloadNode := &yaml.Node{
102+
Kind: yaml.ScalarNode,
103+
Value: "$inputs.userId",
104+
}
105+
validRequestBody.Payload = validPayloadNode
106+
validRequestBody.Model = marshaller.Model[core.RequestBody]{
107+
Valid: true,
108+
}
109+
110+
validationErrors := validRequestBody.Validate(context.Background())
111+
assert.Empty(t, validationErrors, "Valid top-level expression should not produce validation errors")
112+
113+
// Test invalid top-level expression (valid type but invalid format)
114+
invalidRequestBody := &arazzo.RequestBody{}
115+
116+
invalidPayloadNode := &yaml.Node{
117+
Kind: yaml.ScalarNode,
118+
Value: "$inputs", // Missing required name after $inputs
119+
}
120+
invalidRequestBody.Payload = invalidPayloadNode
121+
invalidRequestBody.Model = marshaller.Model[core.RequestBody]{
122+
Valid: true,
123+
}
124+
125+
validationErrors = invalidRequestBody.Validate(context.Background())
126+
assert.NotEmpty(t, validationErrors, "Invalid top-level expression should produce validation errors")
127+
assert.Contains(t, validationErrors[0].Error(), "payload expression is not valid")
128+
}

‎arazzo/reusable.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ func (r *Reusable[T, V, C]) Validate(ctx context.Context, opts ...validation.Opt
132132

133133
func (r *Reusable[T, V, C]) validateReference(ctx context.Context, a *Arazzo, opts ...validation.Option) []error {
134134
core := r.GetCore()
135-
if err := r.Reference.Validate(true); err != nil {
135+
if err := r.Reference.Validate(); err != nil {
136136
return []error{
137137
validation.NewValueError(validation.NewValueValidationError(err.Error()), core, core.Reference),
138138
}

‎arazzo/step.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ func (s *Step) Validate(ctx context.Context, opts ...validation.Option) []error
137137
errs = append(errs, validation.NewValueError(validation.NewValueValidationError("operationId must be a valid expression if there are multiple OpenAPI source descriptions"), core, core.OperationID))
138138
}
139139
if s.OperationID.IsExpression() {
140-
if err := s.OperationID.Validate(false); err != nil {
140+
if err := s.OperationID.Validate(); err != nil {
141141
errs = append(errs, validation.NewValueError(validation.NewValueValidationError(err.Error()), core, core.OperationID))
142142
}
143143

@@ -154,7 +154,7 @@ func (s *Step) Validate(ctx context.Context, opts ...validation.Option) []error
154154
}
155155

156156
if s.OperationPath != nil {
157-
if err := s.OperationPath.Validate(true); err != nil {
157+
if err := s.OperationPath.Validate(); err != nil {
158158
errs = append(errs, validation.NewValueError(validation.NewValueValidationError(err.Error()), core, core.OperationPath))
159159
}
160160

@@ -178,7 +178,7 @@ func (s *Step) Validate(ctx context.Context, opts ...validation.Option) []error
178178

179179
if s.WorkflowID != nil {
180180
if s.WorkflowID.IsExpression() {
181-
if err := s.WorkflowID.Validate(false); err != nil {
181+
if err := s.WorkflowID.Validate(); err != nil {
182182
errs = append(errs, validation.NewValueError(validation.NewValueValidationError(err.Error()), core, core.WorkflowID))
183183
}
184184

@@ -281,7 +281,7 @@ func (s *Step) Validate(ctx context.Context, opts ...validation.Option) []error
281281
errs = append(errs, validation.NewMapKeyError(validation.NewValueValidationError("output name must be a valid name [%s]: %s", outputNameRegex.String(), name), core, core.Outputs, name))
282282
}
283283

284-
if err := output.Validate(true); err != nil {
284+
if err := output.Validate(); err != nil {
285285
errs = append(errs, validation.NewMapValueError(validation.NewValueValidationError(err.Error()), core, core.Outputs, name))
286286
}
287287
}

‎arazzo/successaction.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ func validationActionWorkflowIDAndStepID(ctx context.Context, params validationA
132132
}
133133
if params.workflowID != nil {
134134
if params.workflowID.IsExpression() {
135-
if err := params.workflowID.Validate(false); err != nil {
135+
if err := params.workflowID.Validate(); err != nil {
136136
errs = append(errs, &validation.Error{
137137
UnderlyingError: validation.NewValueValidationError(err.Error()),
138138
Line: params.workflowIDLine,

‎arazzo/workflow.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,11 @@ func (w *Workflow) Validate(ctx context.Context, opts ...validation.Option) []er
8787
}
8888

8989
for i, dependsOn := range w.DependsOn {
90-
if err := dependsOn.Validate(false); err != nil {
91-
errs = append(errs, validation.NewSliceError(validation.NewValueValidationError(err.Error()), core, core.DependsOn, i))
92-
}
93-
9490
if dependsOn.IsExpression() {
91+
if err := dependsOn.Validate(); err != nil {
92+
errs = append(errs, validation.NewSliceError(validation.NewValueValidationError(err.Error()), core, core.DependsOn, i))
93+
}
94+
9595
typ, sourceDescriptionName, _, _ := dependsOn.GetParts()
9696

9797
if typ != expression.ExpressionTypeSourceDescriptions {
@@ -125,7 +125,7 @@ func (w *Workflow) Validate(ctx context.Context, opts ...validation.Option) []er
125125
errs = append(errs, validation.NewMapKeyError(validation.NewValueValidationError("output name must be a valid name [%s]: %s", outputNameRegex.String(), name), core, core.Outputs, name))
126126
}
127127

128-
if err := output.Validate(true); err != nil {
128+
if err := output.Validate(); err != nil {
129129
errs = append(errs, validation.NewMapValueError(validation.NewValueValidationError(err.Error()), core, core.Outputs, name))
130130
}
131131
}

0 commit comments

Comments
 (0)