Skip to content

Commit aa724f4

Browse files
committed
fix(jsonfilter): Sanitize input when parsing filters
Signed-off-by: Javier Rodriguez <javier@chainloop.dev>
1 parent 2bd2179 commit aa724f4

4 files changed

Lines changed: 128 additions & 14 deletions

File tree

app/controlplane/pkg/biz/workflow_integration_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz/testhelpers"
2929
"github.com/chainloop-dev/chainloop/pkg/credentials"
3030
creds "github.com/chainloop-dev/chainloop/pkg/credentials/mocks"
31+
"github.com/chainloop-dev/chainloop/pkg/jsonfilter"
3132
"github.com/google/go-cmp/cmp"
3233
"github.com/google/go-cmp/cmp/cmpopts"
3334
"github.com/google/uuid"
@@ -561,6 +562,35 @@ func (s *workflowListIntegrationTestSuite) TestList() {
561562
s.Len(workflows, 1)
562563
s.Equal(2, count)
563564
})
565+
566+
s.Run("rejects a JSON filter with an unsafe field path as a validation error", func() {
567+
opts := &biz.WorkflowListOpts{
568+
JSONFilters: []*jsonfilter.JSONFilter{
569+
{
570+
// SQL injection attempt: a single quote breaks out of the JSON path literal.
571+
FieldPath: "x'='x' OR (SELECT 1 FROM pg_sleep(2)) IS NOT NULL OR 'z",
572+
Operator: jsonfilter.OpEQ,
573+
Value: "true",
574+
},
575+
},
576+
}
577+
578+
workflows, _, err := s.Workflow.List(ctx, s.org.ID, opts, nil)
579+
s.Error(err)
580+
s.True(biz.IsErrValidation(err), "unsafe field path must surface as a validation error, got: %v", err)
581+
s.Nil(workflows)
582+
})
583+
584+
s.Run("accepts a JSON filter with a safe field path", func() {
585+
opts := &biz.WorkflowListOpts{
586+
JSONFilters: []*jsonfilter.JSONFilter{
587+
{FieldPath: "needsAttention", Operator: jsonfilter.OpEQ, Value: "true"},
588+
},
589+
}
590+
591+
_, _, err := s.Workflow.List(ctx, s.org.ID, opts, nil)
592+
s.NoError(err)
593+
})
564594
}
565595

566596
// Run the tests

app/controlplane/pkg/data/workflow.go

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,10 @@ func (r *WorkflowRepo) List(ctx context.Context, orgID uuid.UUID, filter *biz.Wo
281281
wfQuery := baseQuery.Where(workflow.DeletedAtIsNil())
282282

283283
// Apply additional filters to the Workflow query based on the provided options
284-
wfQuery = applyWorkflowFilters(wfQuery, filter)
284+
wfQuery, err := applyWorkflowFilters(wfQuery, filter)
285+
if err != nil {
286+
return nil, 0, err
287+
}
285288

286289
// Get the count of all filtered rows without the limit and offset
287290
count, err := wfQuery.Count(ctx)
@@ -339,7 +342,7 @@ func applyWorkflowRunFilters(baseQuery *ent.WorkflowQuery, opts *biz.WorkflowLis
339342
}
340343

341344
// applyWorkflowFilters applies filters to the Workflow query based on the provided options
342-
func applyWorkflowFilters(wfQuery *ent.WorkflowQuery, opts *biz.WorkflowListOpts) *ent.WorkflowQuery {
345+
func applyWorkflowFilters(wfQuery *ent.WorkflowQuery, opts *biz.WorkflowListOpts) (*ent.WorkflowQuery, error) {
343346
if opts != nil {
344347
if opts.WorkflowPublic != nil {
345348
wfQuery = wfQuery.Where(workflow.Public(*opts.WorkflowPublic))
@@ -364,16 +367,21 @@ func applyWorkflowFilters(wfQuery *ent.WorkflowQuery, opts *biz.WorkflowListOpts
364367

365368
// Append the JSON Filters to the query
366369
if len(opts.JSONFilters) != 0 {
367-
wfQuery = wfQuery.Where(func(selector *sql.Selector) {
368-
// Build the predicates for each JSON filter
369-
predicates := make([]*sql.Predicate, 0, len(opts.JSONFilters))
370-
for _, filter := range opts.JSONFilters {
371-
// Include the column where the filter is applied
372-
filter.Column = workflow.FieldMetadata
373-
jsonPredicate, _ := jsonfilter.BuildEntSelectorFromJSONFilter(filter)
374-
predicates = append(predicates, jsonPredicate)
370+
// Build the predicates for each JSON filter up front so that any
371+
// validation error (e.g. an unsafe field path) is surfaced instead
372+
// of being silently ignored inside the query builder closure.
373+
predicates := make([]*sql.Predicate, 0, len(opts.JSONFilters))
374+
for _, filter := range opts.JSONFilters {
375+
// Include the column where the filter is applied
376+
filter.Column = workflow.FieldMetadata
377+
jsonPredicate, err := jsonfilter.BuildEntSelectorFromJSONFilter(filter)
378+
if err != nil {
379+
return nil, biz.NewErrValidation(fmt.Errorf("invalid JSON filter: %w", err))
375380
}
376-
// Combine the predicates using OR logic
381+
predicates = append(predicates, jsonPredicate)
382+
}
383+
wfQuery = wfQuery.Where(func(selector *sql.Selector) {
384+
// Combine the predicates using AND logic
377385
selector.Where(sql.And(predicates...))
378386
})
379387
}
@@ -396,7 +404,7 @@ func applyWorkflowFilters(wfQuery *ent.WorkflowQuery, opts *biz.WorkflowListOpts
396404
}
397405
}
398406

399-
return wfQuery
407+
return wfQuery, nil
400408
}
401409

402410
// GetOrgScoped Gets a workflow making sure it belongs to a given org

pkg/jsonfilter/jsonfilter.go

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
//
2-
// Copyright 2025 The Chainloop Authors.
2+
// Copyright 2025-2026 The Chainloop Authors.
33
//
44
// Licensed under the Apache License, Version 2.0 (the "License");
55
// you may not use this file except in compliance with the License.
@@ -18,12 +18,24 @@ package jsonfilter
1818
import (
1919
"errors"
2020
"fmt"
21+
"regexp"
2122
"strings"
2223

2324
entsql "entgo.io/ent/dialect/sql"
2425
"entgo.io/ent/dialect/sql/sqljson"
2526
)
2627

28+
// fieldPathRegexp matches a safe JSON field path expressed in dot notation.
29+
// A path is a dot-separated sequence of identifiers (starting with a letter or
30+
// underscore) optionally followed by numeric array indices, e.g. "name",
31+
// "labels.env" or "items[0].name".
32+
//
33+
// This allowlist is security-critical: the underlying ent sqljson helpers
34+
// concatenate path segments verbatim into single-quoted PostgreSQL JSON path
35+
// literals without escaping, so any character outside this set (e.g. a single
36+
// quote) would allow breaking out of the literal and injecting arbitrary SQL.
37+
var fieldPathRegexp = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*(?:\.[A-Za-z_][A-Za-z0-9_]*|\[[0-9]+\])*$`)
38+
2739
// JSONOperator represents supported JSON filter operators.
2840
type JSONOperator string
2941

@@ -54,6 +66,13 @@ func BuildEntSelectorFromJSONFilter(jsonFilter *JSONFilter) (*entsql.Predicate,
5466
return nil, errors.New("invalid filter: column and operator are required")
5567
}
5668

69+
// Validate the field path before it reaches the SQL builder. The ent
70+
// sqljson helpers concatenate the path segments unescaped into the query,
71+
// so an unsafe value would allow SQL injection.
72+
if err := validateFieldPath(jsonFilter.FieldPath); err != nil {
73+
return nil, err
74+
}
75+
5776
// Convert the dot notation to the path that Ent expects.
5877
dotPath := sqljson.DotPath(jsonFilter.FieldPath)
5978

@@ -85,3 +104,18 @@ func BuildEntSelectorFromJSONFilter(jsonFilter *JSONFilter) (*entsql.Predicate,
85104
return nil, fmt.Errorf("unsupported operator: %s", jsonFilter.Operator)
86105
}
87106
}
107+
108+
// validateFieldPath ensures the JSON field path only contains safe characters
109+
// before it is concatenated into the SQL query by the ent sqljson helpers.
110+
// An empty path is allowed: it targets the column itself and is not injectable.
111+
func validateFieldPath(fieldPath string) error {
112+
if fieldPath == "" {
113+
return nil
114+
}
115+
116+
if !fieldPathRegexp.MatchString(fieldPath) {
117+
return fmt.Errorf("invalid field path %q: must be dot-separated identifiers with optional numeric array indices", fieldPath)
118+
}
119+
120+
return nil
121+
}

pkg/jsonfilter/jsonfilter_test.go

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ import (
2222
"github.com/stretchr/testify/require"
2323
)
2424

25+
// errInvalidFieldPath is the common prefix returned when a field path fails validation.
26+
const errInvalidFieldPath = "invalid field path"
27+
2528
func TestBuildEntSelectorFromJSONFilter(t *testing.T) {
2629
tests := []struct {
2730
name string
@@ -81,13 +84,52 @@ func TestBuildEntSelectorFromJSONFilter(t *testing.T) {
8184
filter: &JSONFilter{Column: "metadata", FieldPath: "env", Operator: OpIN, Value: []string{"prod", "dev"}},
8285
wantErr: "invalid value for 'in' operator: must be a slice of strings",
8386
},
87+
{
88+
name: "field path with array index",
89+
filter: &JSONFilter{Column: "metadata", FieldPath: "items[0].name", Operator: OpEQ, Value: "foo"},
90+
},
91+
{
92+
name: "field path with single quote breaks out of literal",
93+
filter: &JSONFilter{Column: "metadata", FieldPath: "x'='x' OR (SELECT 1 FROM pg_sleep(2)) IS NOT NULL OR 'z", Operator: OpEQ, Value: "true"},
94+
wantErr: errInvalidFieldPath,
95+
},
96+
{
97+
name: "field path with double quote",
98+
filter: &JSONFilter{Column: "metadata", FieldPath: `name"`, Operator: OpEQ, Value: "foo"},
99+
wantErr: errInvalidFieldPath,
100+
},
101+
{
102+
name: "field path with semicolon",
103+
filter: &JSONFilter{Column: "metadata", FieldPath: "name;DROP TABLE workflows", Operator: OpEQ, Value: "foo"},
104+
wantErr: errInvalidFieldPath,
105+
},
106+
{
107+
name: "field path with whitespace",
108+
filter: &JSONFilter{Column: "metadata", FieldPath: "name OR 1=1", Operator: OpEQ, Value: "foo"},
109+
wantErr: errInvalidFieldPath,
110+
},
111+
{
112+
name: "field path with parenthesis",
113+
filter: &JSONFilter{Column: "metadata", FieldPath: "pg_sleep(2)", Operator: OpEQ, Value: "foo"},
114+
wantErr: errInvalidFieldPath,
115+
},
116+
{
117+
name: "field path with leading digit segment",
118+
filter: &JSONFilter{Column: "metadata", FieldPath: "1name", Operator: OpEQ, Value: "foo"},
119+
wantErr: errInvalidFieldPath,
120+
},
121+
{
122+
name: "field path with trailing dot",
123+
filter: &JSONFilter{Column: "metadata", FieldPath: "name.", Operator: OpEQ, Value: "foo"},
124+
wantErr: errInvalidFieldPath,
125+
},
84126
}
85127

86128
for _, tc := range tests {
87129
t.Run(tc.name, func(t *testing.T) {
88130
pred, err := BuildEntSelectorFromJSONFilter(tc.filter)
89131
if tc.wantErr != "" {
90-
require.EqualError(t, err, tc.wantErr)
132+
require.ErrorContains(t, err, tc.wantErr)
91133
assert.Nil(t, pred)
92134
} else {
93135
require.NoError(t, err)

0 commit comments

Comments
 (0)