feat(validation): JSON-Schema compile timeout + depth pre-scan#256
Merged
Conversation
Lands the second half of #217 (security review finding 6): bound the JSON-Schema compiler so a malicious per-field constraint cannot hang the server during the 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.
zeevdr
added
enhancement
This was referenced Apr 30, 2026
zeevdr
added a commit
that referenced
this pull request
May 1, 2026
PR #256 added a wall-clock CompileTimeout to newJSONSchemaValidator. The success and disabled-timeout branches were tested; the timeout-fired branch was not. Add TestNewJSONSchemaValidator_CompileTimeoutFires using a 1ns timeout against a small valid schema. The timer.C is ready before the compile goroutine can be scheduled and push to the buffered result channel, so the select reaches the timeout branch deterministically (verified across 20 consecutive runs). Closes #281 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
zeevdr
added a commit
that referenced
this pull request
May 1, 2026
PR #254 added configurable schema ingest limits (MaxFields, MaxDocBytes) and PR #256 added JSON-Schema compile-time bounds. The validators have unit tests; this lands the live e2e coverage of the user-facing rejection paths. Lower the docker-compose service's caps to small values (SCHEMA_MAX_FIELDS=100, SCHEMA_MAX_DOC_BYTES=4096) so the new tests can trip them with cheap payloads. Existing fixtures use at most a few fields and well under 1 KiB of YAML, so other e2e tests are unaffected. Add three tests in e2e/validation_limits_test.go: - TestValidationLimits_MaxFieldsRejected — CreateSchema with 101 fields returns InvalidArgument and the message cites the configured cap. - TestValidationLimits_MaxFieldsAtLimitAccepted — exactly 100 fields is accepted; the cap is inclusive at the limit. - TestValidationLimits_MaxDocBytesRejected — ImportSchema with a YAML body padded past the cap is rejected before the parser runs. The JSON-Schema MaxDepth path is intentionally not covered here: internal/validation/validator.go silently swallows newJSONSchemaValidator errors at line 147 (constraints.JsonSchema branch), so a depth-exceeded schema is accepted at CreateSchema and only surfaces later as a missing JSON-Schema check. That swallow is worth a separate issue; the depth trigger itself is unit-tested in internal/validation. Closes #283 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
zeevdr
added a commit
that referenced
this pull request
May 1, 2026
PR #256 added a wall-clock CompileTimeout to newJSONSchemaValidator. The success and disabled-timeout branches were tested; the timeout-fired branch was not. Add TestNewJSONSchemaValidator_CompileTimeoutFires using a 1ns timeout against a small valid schema. The timer.C is ready before the compile goroutine can be scheduled and push to the buffered result channel, so the select reaches the timeout branch deterministically (verified across 20 consecutive runs). Closes #281 Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
zeevdr
added a commit
that referenced
this pull request
May 3, 2026
* test(e2e): cover validation-limit rejections (#283) PR #254 added configurable schema ingest limits (MaxFields, MaxDocBytes) and PR #256 added JSON-Schema compile-time bounds. The validators have unit tests; this lands the live e2e coverage of the user-facing rejection paths. Lower the docker-compose service's caps to small values (SCHEMA_MAX_FIELDS=100, SCHEMA_MAX_DOC_BYTES=4096) so the new tests can trip them with cheap payloads. Existing fixtures use at most a few fields and well under 1 KiB of YAML, so other e2e tests are unaffected. Add three tests in e2e/validation_limits_test.go: - TestValidationLimits_MaxFieldsRejected — CreateSchema with 101 fields returns InvalidArgument and the message cites the configured cap. - TestValidationLimits_MaxFieldsAtLimitAccepted — exactly 100 fields is accepted; the cap is inclusive at the limit. - TestValidationLimits_MaxDocBytesRejected — ImportSchema with a YAML body padded past the cap is rejected before the parser runs. The JSON-Schema MaxDepth path is intentionally not covered here: internal/validation/validator.go silently swallows newJSONSchemaValidator errors at line 147 (constraints.JsonSchema branch), so a depth-exceeded schema is accepted at CreateSchema and only surfaces later as a missing JSON-Schema check. That swallow is worth a separate issue; the depth trigger itself is unit-tested in internal/validation. Closes #283 Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
json_schemaconstraint cannot hang the server on first compile.validation.Limits(CompileTimeout,MaxDepth) with a sharedOption/WithLimitspattern — same option type works for bothNewValidatorFactoryandNewFieldValidator.DefaultLimitsis 5 s + depth 64.CompileTimeout. Becausejsonschema/v6has noCompileContext, the timeout is a wall-clock backstop — the goroutine may continue past the deadline, but the pre-scan plus the upstreamMaxDocBytescap (feat(schema): configurable ingest limits + functional options for NewService #254) bound the worst-case work.cmd/serverreadsSCHEMA_COMPILE_TIMEOUT(Go duration) andSCHEMA_MAX_REF_DEPTHenv vars and threads them through the shared validator factory.(Replaces #255, which was auto-closed when its base branch
feat/schema-limitswas deleted on #254 merge.)Test plan
make pre-commit(build, vet, format, lint, test, coverage)internal/validation/json_schema_test.gocover:DefaultLimits, depth-exceeded rejection, depth-disabled (MaxDepth: 0), malformed-JSON falls through to compiler, timeout-zero-is-unbounded, and directscanJSONDepthcases (objects, arrays, non-JSON)factory_concurrent_test.go/validator_bench_test.go/cache_race_test.gocallers continue to compile (zero-opt form preserved)SCHEMA_COMPILE_TIMEOUTandSCHEMA_MAX_REF_DEPTHdocumented indocs/server/configuration.md.agents/context/security-review.mdupdated — finding 6 now marked complete across both halvesCloses #217.
🤖 Generated with Claude Code