Skip to content

Commit ad6470d

Browse files
authored
feat(validation): JSON-Schema compile timeout + depth pre-scan (#256)
Lands the second half of #217: bound the JSON-Schema compiler so a malicious per-field json_schema constraint cannot hang the server on first compile. Sits on top of #254, which already caps schema doc bytes and field count at the ingest layer. - internal/validation/limits.go — new Limits struct (CompileTimeout, MaxDepth) + DefaultLimits (5 s, depth 64) + shared Option/WithLimits pattern usable by both the factory and individual field validators. - internal/validation/json_schema.go — newJSONSchemaValidator now takes Limits. A pre-compile depth scan rejects pathological nesting before invoking the compiler; the Compile call itself runs in a goroutine and is bounded by CompileTimeout. jsonschema/v6 has no CompileContext, so the timeout is a wall-clock backstop — the goroutine may continue past the deadline, but the depth scan + upstream MaxDocBytes cap bound the worst-case work. - NewValidatorFactory(store, opts...) and NewFieldValidator(..., opts...) accept the shared Option type; existing zero-opt call sites continue to compile unchanged. - cmd/server reads SCHEMA_COMPILE_TIMEOUT (Go duration) and SCHEMA_MAX_REF_DEPTH env vars and threads them through WithLimits on the shared validator factory. Closes #217.
1 parent 94e6d2b commit ad6470d

8 files changed

Lines changed: 255 additions & 25 deletions

File tree

.agents/context/security-review.md

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -148,11 +148,18 @@ a unary/stream interceptor.
148148
Fix: cap field count (e.g. 10 000), schema-document bytes (e.g. 5 MB),
149149
and wrap compilation in a context deadline (e.g. 5 s).
150150

151-
Field count + doc bytes landed via `schema.Limits` (`internal/schema/limits.go`),
152-
configurable through `WithLimits` and env vars `SCHEMA_MAX_FIELDS` /
153-
`SCHEMA_MAX_DOC_BYTES`. Compile timeout still pending — needs a refactor
154-
of `internal/validation/json_schema.go` since `jsonschema/v6` has no
155-
`CompileContext`.
151+
All four bounds shipped:
152+
153+
- Field count + doc bytes via `schema.Limits` (`internal/schema/limits.go`),
154+
configurable through `schema.WithLimits` and env vars `SCHEMA_MAX_FIELDS` /
155+
`SCHEMA_MAX_DOC_BYTES`.
156+
- Compile timeout + structural depth scan via `validation.Limits`
157+
(`internal/validation/limits.go`), configurable through
158+
`validation.WithLimits` and env vars `SCHEMA_COMPILE_TIMEOUT` /
159+
`SCHEMA_MAX_REF_DEPTH`. Because `jsonschema/v6` has no `CompileContext`,
160+
the timeout is a goroutine-level wrapper — the underlying compile may
161+
continue past the deadline, but the depth pre-scan and upstream doc-byte
162+
cap bound the worst-case work.
156163

157164
### 7. Audit log not tamper-evident — High
158165

cmd/server/main.go

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,12 @@ func run() int {
210210
schemaMetrics := telemetry.NewSchemaMetrics(otelCfg)
211211

212212
// Validator factory (shared between schema + config services).
213-
validatorFactory := validation.NewValidatorFactory(validatorStore)
213+
validatorFactory := validation.NewValidatorFactory(validatorStore,
214+
validation.WithLimits(validation.Limits{
215+
CompileTimeout: cfg.SchemaCompileTimeout,
216+
MaxDepth: cfg.SchemaMaxRefDepth,
217+
}),
218+
)
214219

215220
// Usage recorder — async batched read tracking for audit stats.
216221
var recorder *audit.UsageRecorder
@@ -352,6 +357,8 @@ type serverConfig struct {
352357
GRPCMaxSendMsgBytes int
353358
SchemaMaxFields int
354359
SchemaMaxDocBytes int
360+
SchemaCompileTimeout time.Duration
361+
SchemaMaxRefDepth int
355362
InsecureListen bool
356363
TLSCertFile string
357364
TLSKeyFile string
@@ -389,6 +396,16 @@ func loadConfig() serverConfig {
389396
}
390397
}
391398

399+
compileTimeout := 5 * time.Second
400+
if v := getEnv("SCHEMA_COMPILE_TIMEOUT", ""); v != "" {
401+
d, err := time.ParseDuration(v)
402+
if err != nil {
403+
slog.Error("invalid duration env var", "key", "SCHEMA_COMPILE_TIMEOUT", "value", v, "error", err)
404+
os.Exit(1)
405+
}
406+
compileTimeout = d
407+
}
408+
392409
return serverConfig{
393410
GRPCPort: getEnv("GRPC_PORT", "9090"),
394411
HTTPPort: getEnv("HTTP_PORT", ""),
@@ -406,6 +423,8 @@ func loadConfig() serverConfig {
406423
GRPCMaxSendMsgBytes: parseEnvInt("GRPC_MAX_SEND_MSG_BYTES", 0),
407424
SchemaMaxFields: parseEnvInt("SCHEMA_MAX_FIELDS", 10_000),
408425
SchemaMaxDocBytes: parseEnvInt("SCHEMA_MAX_DOC_BYTES", 5*1024*1024),
426+
SchemaCompileTimeout: compileTimeout,
427+
SchemaMaxRefDepth: parseEnvInt("SCHEMA_MAX_REF_DEPTH", 64),
409428
InsecureListen: getEnv("INSECURE_LISTEN", "") == "1",
410429
TLSCertFile: getEnv("TLS_CERT_FILE", ""),
411430
TLSKeyFile: getEnv("TLS_KEY_FILE", ""),

docs/server/configuration.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ OpenDecree is configured entirely through environment variables. No config files
2020
| `GRPC_MAX_SEND_MSG_BYTES` | Maximum size of an outbound gRPC message, in bytes. Responses above this return `ResourceExhausted` to the client. Set to `0` for the default. | `20971520` (20 MiB) | No |
2121
| `SCHEMA_MAX_FIELDS` | Maximum number of fields per schema accepted by `CreateSchema` and `ImportSchema`. Requests above this return `InvalidArgument`. Set to `0` to disable. | `10000` | No |
2222
| `SCHEMA_MAX_DOC_BYTES` | Maximum serialized YAML document size accepted by `ImportSchema`, in bytes. Requests above this return `InvalidArgument`. Set to `0` to disable. | `5242880` (5 MiB) | No |
23+
| `SCHEMA_COMPILE_TIMEOUT` | Wall-clock cap on a single JSON-Schema compile (per-field constraint). Format: Go duration (e.g., `5s`, `2s`). Set to `0` to disable the timeout. | `5s` | No |
24+
| `SCHEMA_MAX_REF_DEPTH` | Maximum structural nesting depth of a JSON-Schema constraint document. Schemas deeper than this are rejected before compilation. Set to `0` to disable. | `64` | No |
2325

2426
## Transport Security (TLS)
2527

internal/validation/factory.go

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,17 @@ type ValidatorFactory struct {
2424
store Store
2525
cache *ValidatorCache
2626
rulesCache sync.Map // tenantID → []byte (raw dependent_required JSON)
27+
limits Limits
2728
}
2829

29-
// NewValidatorFactory creates a new validator factory.
30-
func NewValidatorFactory(store Store) *ValidatorFactory {
30+
// NewValidatorFactory creates a new validator factory. Pass [WithLimits]
31+
// to override the JSON-Schema compile defaults.
32+
func NewValidatorFactory(store Store, opts ...Option) *ValidatorFactory {
33+
o := resolveOptions(opts)
3134
return &ValidatorFactory{
32-
store: store,
33-
cache: NewValidatorCache(0),
35+
store: store,
36+
cache: NewValidatorCache(0),
37+
limits: o.limits,
3438
}
3539
}
3640

@@ -107,7 +111,7 @@ func (f *ValidatorFactory) GetValidators(ctx context.Context, tenantID string) (
107111
constraints = &pb.FieldConstraints{}
108112
_ = json.Unmarshal(field.Constraints, constraints)
109113
}
110-
validators[field.Path] = NewFieldValidator(field.Path, ft, field.Nullable, constraints)
114+
validators[field.Path] = NewFieldValidator(field.Path, ft, field.Nullable, constraints, WithLimits(f.limits))
111115
}
112116

113117
f.cache.Set(tenantID, validators)

internal/validation/json_schema.go

Lines changed: 83 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
package validation
22

33
import (
4+
"encoding/json"
45
"fmt"
56
"strings"
7+
"time"
68

79
"github.com/santhosh-tekuri/jsonschema/v6"
810
)
@@ -13,20 +15,57 @@ type jsonSchemaValidator struct {
1315
}
1416

1517
// newJSONSchemaValidator compiles a JSON Schema document for validation.
16-
func newJSONSchemaValidator(schemaDoc string) (*jsonSchemaValidator, error) {
17-
c := jsonschema.NewCompiler()
18-
doc, err := jsonschema.UnmarshalJSON(strings.NewReader(schemaDoc))
19-
if err != nil {
20-
return nil, fmt.Errorf("invalid json schema: %w", err)
18+
// limits.MaxDepth bounds structural nesting before compile;
19+
// limits.CompileTimeout caps the wall-clock duration of the compile call.
20+
//
21+
// Note: jsonschema/v6 has no CompileContext, so a compile that exceeds the
22+
// deadline will continue running in its goroutine until it finishes (or
23+
// the process exits). The pre-compile depth check and upstream document-
24+
// size cap (schema.Limits.MaxDocBytes) bound the worst-case work; the
25+
// timeout is a defense-in-depth backstop against unanticipated pathologies.
26+
func newJSONSchemaValidator(schemaDoc string, limits Limits) (*jsonSchemaValidator, error) {
27+
if limits.MaxDepth > 0 {
28+
if err := scanJSONDepth(schemaDoc, limits.MaxDepth); err != nil {
29+
return nil, err
30+
}
2131
}
22-
if err := c.AddResource("schema.json", doc); err != nil {
23-
return nil, fmt.Errorf("add json schema resource: %w", err)
32+
33+
type result struct {
34+
v *jsonSchemaValidator
35+
err error
2436
}
25-
schema, err := c.Compile("schema.json")
26-
if err != nil {
27-
return nil, fmt.Errorf("compile json schema: %w", err)
37+
ch := make(chan result, 1)
38+
go func() {
39+
c := jsonschema.NewCompiler()
40+
doc, err := jsonschema.UnmarshalJSON(strings.NewReader(schemaDoc))
41+
if err != nil {
42+
ch <- result{nil, fmt.Errorf("invalid json schema: %w", err)}
43+
return
44+
}
45+
if err := c.AddResource("schema.json", doc); err != nil {
46+
ch <- result{nil, fmt.Errorf("add json schema resource: %w", err)}
47+
return
48+
}
49+
schema, err := c.Compile("schema.json")
50+
if err != nil {
51+
ch <- result{nil, fmt.Errorf("compile json schema: %w", err)}
52+
return
53+
}
54+
ch <- result{&jsonSchemaValidator{schema: schema}, nil}
55+
}()
56+
57+
if limits.CompileTimeout <= 0 {
58+
r := <-ch
59+
return r.v, r.err
60+
}
61+
timer := time.NewTimer(limits.CompileTimeout)
62+
defer timer.Stop()
63+
select {
64+
case r := <-ch:
65+
return r.v, r.err
66+
case <-timer.C:
67+
return nil, fmt.Errorf("compile json schema: timeout after %s", limits.CompileTimeout)
2868
}
29-
return &jsonSchemaValidator{schema: schema}, nil
3069
}
3170

3271
// validate checks a JSON string against the compiled schema.
@@ -40,3 +79,36 @@ func (v *jsonSchemaValidator) validate(jsonStr string) error {
4079
}
4180
return nil
4281
}
82+
83+
// scanJSONDepth walks the parsed JSON document and returns an error if
84+
// nesting exceeds maxDepth. Non-JSON input is ignored and left for the
85+
// compiler to report — this scan exists to short-circuit obvious bombs,
86+
// not to validate syntax.
87+
func scanJSONDepth(doc string, maxDepth int) error {
88+
var v any
89+
if jsonErr := json.Unmarshal([]byte(doc), &v); jsonErr != nil {
90+
return nil //nolint:nilerr // non-JSON input is intentionally left for the compiler to report
91+
}
92+
return checkDepth(v, 0, maxDepth)
93+
}
94+
95+
func checkDepth(v any, depth, max int) error {
96+
if depth > max {
97+
return fmt.Errorf("compile json schema: nesting depth exceeds limit of %d", max)
98+
}
99+
switch t := v.(type) {
100+
case map[string]any:
101+
for _, child := range t {
102+
if err := checkDepth(child, depth+1, max); err != nil {
103+
return err
104+
}
105+
}
106+
case []any:
107+
for _, child := range t {
108+
if err := checkDepth(child, depth+1, max); err != nil {
109+
return err
110+
}
111+
}
112+
}
113+
return nil
114+
}
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
package validation
2+
3+
import (
4+
"strings"
5+
"testing"
6+
"time"
7+
8+
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
10+
)
11+
12+
func TestDefaultLimits(t *testing.T) {
13+
l := DefaultLimits()
14+
assert.Equal(t, 5*time.Second, l.CompileTimeout)
15+
assert.Equal(t, 64, l.MaxDepth)
16+
}
17+
18+
func TestNewJSONSchemaValidator_Compiles(t *testing.T) {
19+
doc := `{"type":"object","properties":{"name":{"type":"string"}}}`
20+
v, err := newJSONSchemaValidator(doc, DefaultLimits())
21+
require.NoError(t, err)
22+
require.NotNil(t, v)
23+
require.NoError(t, v.validate(`{"name":"x"}`))
24+
require.Error(t, v.validate(`{"name":1}`))
25+
}
26+
27+
func TestNewJSONSchemaValidator_DepthExceeded(t *testing.T) {
28+
// Build a schema with nesting depth 10, then cap MaxDepth to 5.
29+
doc := strings.Repeat(`{"properties":{"x":`, 10) + `{"type":"string"}` + strings.Repeat(`}}`, 10)
30+
_, err := newJSONSchemaValidator(doc, Limits{MaxDepth: 5})
31+
require.Error(t, err)
32+
assert.Contains(t, err.Error(), "nesting depth exceeds limit of 5")
33+
}
34+
35+
func TestNewJSONSchemaValidator_DepthDisabled(t *testing.T) {
36+
doc := strings.Repeat(`{"properties":{"x":`, 5) + `{"type":"string"}` + strings.Repeat(`}}`, 5)
37+
v, err := newJSONSchemaValidator(doc, Limits{MaxDepth: 0})
38+
require.NoError(t, err)
39+
require.NotNil(t, v)
40+
}
41+
42+
func TestNewJSONSchemaValidator_MalformedJSONFallsThrough(t *testing.T) {
43+
// Pre-scan ignores non-JSON; compiler reports the syntax error.
44+
_, err := newJSONSchemaValidator(`not-json`, DefaultLimits())
45+
require.Error(t, err)
46+
assert.Contains(t, err.Error(), "invalid json schema")
47+
}
48+
49+
func TestNewJSONSchemaValidator_TimeoutZeroIsUnbounded(t *testing.T) {
50+
doc := `{"type":"string"}`
51+
v, err := newJSONSchemaValidator(doc, Limits{CompileTimeout: 0, MaxDepth: 0})
52+
require.NoError(t, err)
53+
require.NotNil(t, v)
54+
}
55+
56+
func TestScanJSONDepth(t *testing.T) {
57+
// Object nesting.
58+
require.NoError(t, scanJSONDepth(`{"a":{"b":{"c":1}}}`, 5))
59+
require.Error(t, scanJSONDepth(`{"a":{"b":{"c":1}}}`, 2))
60+
61+
// Array nesting counts too.
62+
require.NoError(t, scanJSONDepth(`[[[[1]]]]`, 5))
63+
require.Error(t, scanJSONDepth(`[[[[1]]]]`, 3))
64+
65+
// Non-JSON: scan is a no-op.
66+
require.NoError(t, scanJSONDepth(`not json`, 0))
67+
}

internal/validation/limits.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
package validation
2+
3+
import "time"
4+
5+
// Limits caps JSON-Schema compilation cost. Zero values mean "no limit"
6+
// for that dimension. Use [DefaultLimits] for safe defaults.
7+
//
8+
// These guard against pathological JSON Schema documents (cyclic $ref,
9+
// exponential allOf/anyOf, extreme nesting) that would otherwise hang or
10+
// OOM the server during the first compile. Tracked in
11+
// opendecree/decree#217 (security review finding 6).
12+
type Limits struct {
13+
// CompileTimeout caps the wall-clock duration of a single
14+
// jsonschema.Compile call. Because jsonschema/v6 has no
15+
// CompileContext, the underlying goroutine may continue running
16+
// past the deadline; the bound on input depth and document size
17+
// (enforced upstream) keeps the worst-case work finite.
18+
CompileTimeout time.Duration
19+
20+
// MaxDepth caps the structural nesting of the schema document
21+
// before compilation. A schema deeper than this is rejected
22+
// without invoking the compiler.
23+
MaxDepth int
24+
}
25+
26+
// DefaultLimits returns conservative defaults: a 5-second compile
27+
// timeout and a max nesting depth of 64. Tune via env vars at the call
28+
// site (cmd/server).
29+
func DefaultLimits() Limits {
30+
return Limits{
31+
CompileTimeout: 5 * time.Second,
32+
MaxDepth: 64,
33+
}
34+
}
35+
36+
// Option configures a [ValidatorFactory] or [FieldValidator]. See
37+
// [WithLimits].
38+
type Option func(*options)
39+
40+
type options struct {
41+
limits Limits
42+
}
43+
44+
// WithLimits sets the JSON-Schema compile limits. Defaults to
45+
// [DefaultLimits] when unset.
46+
func WithLimits(l Limits) Option {
47+
return func(o *options) { o.limits = l }
48+
}
49+
50+
func resolveOptions(opts []Option) options {
51+
o := options{limits: DefaultLimits()}
52+
for _, opt := range opts {
53+
opt(&o)
54+
}
55+
return o
56+
}

internal/validation/validator.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,11 @@ func (v *FieldValidator) Validate(tv *pb.TypedValue) error {
5959
return nil
6060
}
6161

62-
// NewFieldValidator creates a validator for a schema field.
63-
func NewFieldValidator(fieldPath string, fieldType pb.FieldType, nullable bool, constraints *pb.FieldConstraints) *FieldValidator {
62+
// NewFieldValidator creates a validator for a schema field. Pass
63+
// [WithLimits] to override the JSON-Schema compile defaults; without it,
64+
// [DefaultLimits] is used.
65+
func NewFieldValidator(fieldPath string, fieldType pb.FieldType, nullable bool, constraints *pb.FieldConstraints, opts ...Option) *FieldValidator {
66+
o := resolveOptions(opts)
6467
v := &FieldValidator{
6568
fieldPath: fieldPath,
6669
fieldType: fieldType,
@@ -140,7 +143,7 @@ func NewFieldValidator(fieldPath string, fieldType pb.FieldType, nullable bool,
140143

141144
case pb.FieldType_FIELD_TYPE_JSON:
142145
if constraints.JsonSchema != nil {
143-
jv, err := newJSONSchemaValidator(*constraints.JsonSchema)
146+
jv, err := newJSONSchemaValidator(*constraints.JsonSchema, o.limits)
144147
if err == nil {
145148
v.checks = append(v.checks, func(tv *pb.TypedValue) error {
146149
val := tv.Kind.(*pb.TypedValue_JsonValue).JsonValue

0 commit comments

Comments
 (0)