From 48b4686c14b69753521962eba0cc4c086feaab47 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Mon, 15 Jun 2026 12:03:11 +0200 Subject: [PATCH] fix(jsonfilter): validate column identifier before building selector Restrict the JSON filter column to a bare SQL identifier so it cannot reach the query builder unvalidated, mirroring the existing field-path allowlist. The sole current caller hardcodes the column, so this is defense-in-depth that removes reliance on every caller doing so. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: e83297c9-6593-4d73-9315-9547d86beb70 --- pkg/jsonfilter/jsonfilter.go | 29 +++++++++++++++++++++++++ pkg/jsonfilter/jsonfilter_test.go | 35 +++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/pkg/jsonfilter/jsonfilter.go b/pkg/jsonfilter/jsonfilter.go index 998d66829..2ff52fe4b 100644 --- a/pkg/jsonfilter/jsonfilter.go +++ b/pkg/jsonfilter/jsonfilter.go @@ -36,6 +36,16 @@ import ( // quote) would allow breaking out of the literal and injecting arbitrary SQL. var fieldPathRegexp = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*(?:\.[A-Za-z_][A-Za-z0-9_]*|\[[0-9]+\])*$`) +// columnRegexp matches a safe SQL column identifier (a single unqualified +// identifier such as "metadata"). +// +// This allowlist is security-critical: the underlying ent sqljson helpers emit +// the column verbatim into the query and, for PostgreSQL, a column containing a +// double quote bypasses ent's identifier quoting entirely and is written raw, +// allowing SQL injection identical to the field-path case. Restricting the +// column to a bare identifier removes every character an injection would need. +var columnRegexp = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`) + // JSONOperator represents supported JSON filter operators. type JSONOperator string @@ -66,6 +76,13 @@ func BuildEntSelectorFromJSONFilter(jsonFilter *JSONFilter) (*entsql.Predicate, return nil, errors.New("invalid filter: column and operator are required") } + // Validate the column before it reaches the SQL builder. Like the field + // path, the column is emitted into the query by the ent sqljson helpers + // without reliable escaping, so an unsafe value would allow SQL injection. + if err := validateColumn(jsonFilter.Column); err != nil { + return nil, err + } + // Validate the field path before it reaches the SQL builder. The ent // sqljson helpers concatenate the path segments unescaped into the query, // so an unsafe value would allow SQL injection. @@ -105,6 +122,18 @@ func BuildEntSelectorFromJSONFilter(jsonFilter *JSONFilter) (*entsql.Predicate, } } +// validateColumn ensures the JSON column is a bare SQL identifier before it is +// emitted into the query by the ent sqljson helpers. Callers are expected to set +// it to a known column constant, but validating here keeps safety from depending +// on every caller remembering to do so. +func validateColumn(column string) error { + if !columnRegexp.MatchString(column) { + return fmt.Errorf("invalid column %q: must be a valid identifier", column) + } + + return nil +} + // validateFieldPath ensures the JSON field path only contains safe characters // before it is concatenated into the SQL query by the ent sqljson helpers. // An empty path is allowed: it targets the column itself and is not injectable. diff --git a/pkg/jsonfilter/jsonfilter_test.go b/pkg/jsonfilter/jsonfilter_test.go index 20c57f586..a168766e3 100644 --- a/pkg/jsonfilter/jsonfilter_test.go +++ b/pkg/jsonfilter/jsonfilter_test.go @@ -25,6 +25,9 @@ import ( // errInvalidFieldPath is the common prefix returned when a field path fails validation. const errInvalidFieldPath = "invalid field path" +// errInvalidColumn is the common prefix returned when a column fails validation. +const errInvalidColumn = "invalid column" + func TestBuildEntSelectorFromJSONFilter(t *testing.T) { tests := []struct { name string @@ -46,6 +49,38 @@ func TestBuildEntSelectorFromJSONFilter(t *testing.T) { filter: &JSONFilter{Column: "metadata", Operator: "gt", Value: "foo"}, wantErr: "unsupported operator: gt", }, + { + // A double quote in the column breaks out of the identifier quoting + // performed by ent's builder, allowing raw SQL injection. + name: "column with double quote breaks out of identifier", + filter: &JSONFilter{Column: `metadata" OR "1"="1`, FieldPath: "name", Operator: OpEQ, Value: "foo"}, + wantErr: errInvalidColumn, + }, + { + name: "column with single quote", + filter: &JSONFilter{Column: `metadata' OR '1'='1`, FieldPath: "name", Operator: OpEQ, Value: "foo"}, + wantErr: errInvalidColumn, + }, + { + name: "column with whitespace", + filter: &JSONFilter{Column: "metadata OR 1=1", FieldPath: "name", Operator: OpEQ, Value: "foo"}, + wantErr: errInvalidColumn, + }, + { + name: "column with parenthesis", + filter: &JSONFilter{Column: "pg_sleep(2)", FieldPath: "name", Operator: OpEQ, Value: "foo"}, + wantErr: errInvalidColumn, + }, + { + name: "column starting with digit", + filter: &JSONFilter{Column: "1metadata", FieldPath: "name", Operator: OpEQ, Value: "foo"}, + wantErr: errInvalidColumn, + }, + { + name: "column with dot qualifier", + filter: &JSONFilter{Column: "workflow.metadata", FieldPath: "name", Operator: OpEQ, Value: "foo"}, + wantErr: errInvalidColumn, + }, { name: "eq operator with string value", filter: &JSONFilter{Column: "metadata", FieldPath: "name", Operator: OpEQ, Value: "foo"},