feat(schema): configurable ingest limits + functional options for NewService#254
Merged
Conversation
…Service Lands the first half of #217: bound schema document size and field count at CreateSchema / ImportSchema, so a single malicious import cannot exhaust the server before validator caching helps. Compile-timeout for the JSON-Schema compiler is deferred to a follow-up that touches internal/validation. - internal/schema/limits.go — new Limits struct (MaxFields, MaxDocBytes) + DefaultLimits (10 000 fields, 5 MiB). - schema.NewService(store, opts...) — store stays positional; logger, metrics, validators, and limits move to With...() options. Mirrors the config / audit / auth / server functional-options sweep from #235, #236, #249. - ImportSchema rejects YAML payloads above MaxDocBytes before parsing, and CreateSchema / ImportSchema reject schemas above MaxFields after parsing. Zero on either dimension means unlimited. - cmd/server reads SCHEMA_MAX_FIELDS and SCHEMA_MAX_DOC_BYTES env vars via the existing parseEnvInt helper and feeds them through WithLimits. Both are documented in docs/server/configuration.md. Refs #217.
zeevdr
added
enhancement
5 tasks
zeevdr
added a commit
that referenced
this pull request
Apr 29, 2026
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.
5 tasks
zeevdr
added a commit
that referenced
this pull request
Apr 29, 2026
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.
This was referenced Apr 30, 2026
zeevdr
added a commit
that referenced
this pull request
May 1, 2026
) PRs #235, #236, #249, #254 replaced struct-config constructors with functional options across server, gateway, audit, auth, config, adminclient, and schema. The Config / ServiceConfig / RecorderConfig / GatewayConfig structs are removed without backward-compat shims (alpha). This document records before/after for each affected constructor and the list of With...() options, so SDK consumers and embedders know what to update when bumping versions. Closes #275 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
The functional-options refactors (PR #235 server, #236 audit/auth, #249 config/adminclient, #254 schema) made every constructor in cmd/server/main.go take a list of With...() options derived from env vars + cfg flags. The wiring had no tests, so the bug class "flag is read but never wired into an option" was silent. Extract the server + gateway option-construction logic into pure helpers (buildServerOptions, buildGatewayOptions) that return both the option slice and a small struct describing the boolean decisions (UseTLS, UseInsecure, HasRateLimiter). Tests assert on the decisions, catching the silent-default bug class without inspecting opaque option closures. Cases covered: TLS vs Insecure (server + gateway); rate limiter wired when non-nil and absent when nil; extra grpc.ServerOptions are folded into a single WithGRPCServerOptions and don't inflate the option slice. Closes #284 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
zeevdr
added a commit
that referenced
this pull request
May 1, 2026
) (#323) PRs #235, #236, #249, #254 replaced struct-config constructors with functional options across server, gateway, audit, auth, config, adminclient, and schema. The Config / ServiceConfig / RecorderConfig / GatewayConfig structs are removed without backward-compat shims (alpha). This document records before/after for each affected constructor and the list of With...() options, so SDK consumers and embedders know what to update when bumping versions. Closes #275 Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
zeevdr
added a commit
that referenced
this pull request
May 1, 2026
…325) The functional-options refactors (PR #235 server, #236 audit/auth, #249 config/adminclient, #254 schema) made every constructor in cmd/server/main.go take a list of With...() options derived from env vars + cfg flags. The wiring had no tests, so the bug class "flag is read but never wired into an option" was silent. Extract the server + gateway option-construction logic into pure helpers (buildServerOptions, buildGatewayOptions) that return both the option slice and a small struct describing the boolean decisions (UseTLS, UseInsecure, HasRateLimiter). Tests assert on the decisions, catching the silent-default bug class without inspecting opaque option closures. Cases covered: TLS vs Insecure (server + gateway); rate limiter wired when non-nil and absent when nil; extra grpc.ServerOptions are folded into a single WithGRPCServerOptions and don't inflate the option slice. Closes #284 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
CreateSchema/ImportSchema, so a single malicious import cannot exhaust the server before validator caching helps.schema.Limitsstruct (MaxFields,MaxDocBytes) wired through a freshWithLimitsoption.DefaultLimitsis 10 000 fields / 5 MiB.schema.NewServiceto functional options (matches the recentconfig/audit/auth/serversweep from refactor(server): functional options for New + NewGateway #235 / refactor(audit,auth): functional options for NewUsageRecorder + NewInterceptor #236 / refactor(config,adminclient): functional options for NewService + New #249) — onlystorestays positional.internal/validation).Test plan
make pre-commit(build, vet, format, lint, test, coverage)internal/schema/limits_test.gocover:DefaultLimits,CreateSchemaoverMaxFields,CreateSchemaexactly at limit,ImportSchemaoverMaxDocBytes,ImportSchemaoverMaxFieldsafter parsingservice_test.go/service_extended_test.gomigrated to the new constructor signature and still passcmd/servercallsite updated;internal/server/memory_integration_test.gomigrated and passesSCHEMA_MAX_FIELDSandSCHEMA_MAX_DOC_BYTESdocumented indocs/server/configuration.md.agents/context/security-review.mdupdated with progress note on schema: bound field count, doc size, JSON-Schema compile time #217Refs #217. Merge before #255.
🤖 Generated with Claude Code