feat: add skip_validation custom hook for value-level bypass#103
feat: add skip_validation custom hook for value-level bypass#103
Conversation
📝 WalkthroughWalkthroughAdds a Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller as Caller
participant Validator as GeneratedValidator
participant Custom as customlib.skip_validation
participant Schema as Schema
Caller->>Validator: validate(value)
Validator->>Custom: skip_validation(value, Schema)
alt predicate returns true
Custom-->>Validator: true
Validator-->>Caller: return true
else predicate returns false
Custom-->>Validator: false
Validator->>Validator: run type & constraint checks
Validator-->>Caller: return validation_result
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Adds an opt-in skip_validation hook to generated validators so specific values can bypass all schema constraints (useful for placeholder/secret reference strings).
Changes:
- Plumbs a
skip_validationcallback throughgenerate_validator()options/custom hooks. - Emits an early-return check in each generated sub-validator when the hook is configured.
- Adds test cases validating bypass behavior across types, enums, nested objects, arrays, defaults, and the no-hook baseline.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| t/default.lua | Adds new test cases covering skip_validation behavior and baseline behavior without the hook. |
| lib/jsonschema.lua | Stores the hook on the root codegen context, exposes it via custom, and injects the early-return check in generated validator code. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/jsonschema.lua (1)
1214-1221:⚠️ Potential issue | 🟡 MinorValidate
custom.skip_validationtype at API boundary.If callers pass a truthy non-function (e.g.,
true), generated validators will fail at runtime with a function-call error instead of failing fast with a clear message.🔧 Suggested fix
return { generate_validator = function(schema, custom) + if custom ~= nil and custom.skip_validation ~= nil + and type(custom.skip_validation) ~= "function" then + error('custom.skip_validation must be a function') + end + local customlib = { null = custom and custom.null or default_null, match_pattern = custom and custom.match_pattern or match_pattern, parse_ipv4 = custom and custom.parse_ipv4 or parse_ipv4, parse_ipv6 = custom and custom.parse_ipv6 or parse_ipv6, skip_validation = custom and custom.skip_validation or nil, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/jsonschema.lua` around lines 1214 - 1221, In generate_validator, validate the API boundary for custom.skip_validation before adding it to customlib: if custom and custom.skip_validation is not nil ensure type(custom.skip_validation) == "function" and otherwise raise a clear error (e.g., error("custom.skip_validation must be a function")), then assign customlib.skip_validation = custom.skip_validation or nil; this ensures generate_validator and the customlib table (used by the validator) never receive a truthy non-function and fail fast with a helpful message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@lib/jsonschema.lua`:
- Around line 1214-1221: In generate_validator, validate the API boundary for
custom.skip_validation before adding it to customlib: if custom and
custom.skip_validation is not nil ensure type(custom.skip_validation) ==
"function" and otherwise raise a clear error (e.g.,
error("custom.skip_validation must be a function")), then assign
customlib.skip_validation = custom.skip_validation or nil; this ensures
generate_validator and the customlib table (used by the validator) never receive
a truthy non-function and fail fast with a helpful message.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 658c12fe-7df5-40d9-8902-f7c82f11b139
📒 Files selected for processing (2)
lib/jsonschema.luat/default.lua
56fccce to
9775783
Compare
Changes since v0.9.12: - feat: add skip_validation custom hook for value-level bypass
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/jsonschema.lua (2)
614-633:⚠️ Potential issue | 🟡 Minor
skip_validationis not applied to boolean schemas.Because Lines 614-620 return before the hook block,
schema == false/schema == truesub-validators never runskip_validation, which conflicts with “early check at start of each sub-validator”.🔧 Suggested fix
- if schema == true then - ctx:stmt('do return true end') - return ctx - elseif schema == false then - ctx:stmt('do return false, "expect false always" end') - return ctx - end - - -- skip_validation hook: if the caller provided a predicate via + -- skip_validation hook: if the caller provided a predicate via -- custom.skip_validation, check it before any constraint. When it -- returns true the value is accepted as-is (useful for placeholder -- strings like secret references that will be resolved at runtime). -- The predicate receives (value, schema) so it can inspect the -- expected type of the field being validated. if ctx._root._skip_validation then local schema_ref = ctx:uservalue(schema) ctx:stmt(sformat('if %s ~= nil and %s(%s, %s) then return true end', ctx:param(1), ctx:libfunc('custom.skip_validation'), ctx:param(1), schema_ref)) end + + if schema == true then + ctx:stmt('do return true end') + return ctx + elseif schema == false then + ctx:stmt('do return false, "expect false always" end') + return ctx + end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/jsonschema.lua` around lines 614 - 633, The boolean-schema branches (the schema == true / schema == false early returns) bypass the skip-validation hook ctx._root._skip_validation; change the logic so the skip-validation check using ctx:uservalue(schema) and ctx:libfunc('custom.skip_validation') runs before returning for boolean schemas (either move the entire if ctx._root._skip_validation block above the schema == true/false checks or add an equivalent skip-validation invocation into the schema == true and schema == false handling), ensuring you still emit the same final ctx:stmt('do return true end') or ctx:stmt('do return false, "expect false always" end') when the predicate does not short-circuit.
1218-1225:⚠️ Potential issue | 🟠 MajorValidate
custom.skip_validationtype before wiring it.If a truthy non-function is passed (e.g.,
trueor a table), the generated validator code will attempt to call it at line 632 and crash at runtime. Add a type guard at construction time.🔧 Suggested fix
return { generate_validator = function(schema, custom) + if custom ~= nil and custom.skip_validation ~= nil + and type(custom.skip_validation) ~= "function" then + error("custom.skip_validation must be a function") + end + local customlib = { null = custom and custom.null or default_null, match_pattern = custom and custom.match_pattern or match_pattern, parse_ipv4 = custom and custom.parse_ipv4 or parse_ipv4, parse_ipv6 = custom and custom.parse_ipv6 or parse_ipv6, skip_validation = custom and custom.skip_validation or nil, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/jsonschema.lua` around lines 1218 - 1225, generate_validator currently wires custom.skip_validation directly which can be a non-function and later gets called (causing a runtime crash); change the construction of customlib in generate_validator to validate that custom.skip_validation is a function and only assign it when type(custom.skip_validation) == "function" (otherwise set skip_validation to nil) so subsequent calls to skip_validation are safe.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@lib/jsonschema.lua`:
- Around line 614-633: The boolean-schema branches (the schema == true / schema
== false early returns) bypass the skip-validation hook
ctx._root._skip_validation; change the logic so the skip-validation check using
ctx:uservalue(schema) and ctx:libfunc('custom.skip_validation') runs before
returning for boolean schemas (either move the entire if
ctx._root._skip_validation block above the schema == true/false checks or add an
equivalent skip-validation invocation into the schema == true and schema ==
false handling), ensuring you still emit the same final ctx:stmt('do return true
end') or ctx:stmt('do return false, "expect false always" end') when the
predicate does not short-circuit.
- Around line 1218-1225: generate_validator currently wires
custom.skip_validation directly which can be a non-function and later gets
called (causing a runtime crash); change the construction of customlib in
generate_validator to validate that custom.skip_validation is a function and
only assign it when type(custom.skip_validation) == "function" (otherwise set
skip_validation to nil) so subsequent calls to skip_validation are safe.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a6fcfa6f-cda0-4532-8053-e77661865191
📒 Files selected for processing (2)
lib/jsonschema.luat/default.lua
🚧 Files skipped from review as they are similar to previous changes (1)
- t/default.lua
9775783 to
6591904
Compare
Changes since v0.9.12: - feat: add skip_validation custom hook for value-level bypass
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
t/default.lua (1)
403-406: Rename this case toalways-skip hook.The heading says "schema-aware", but the predicate below ignores
schemaand demonstrates the opposite behavior. Aligning the label with the implementation will make the two supported strategies easier to distinguish.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@t/default.lua` around lines 403 - 406, The test case heading/comment is mislabeled as "schema-aware hook" but the predicate ignores schema and implements the opposite behavior; update the test case heading/comment for test case 17 to "always-skip hook" (or similar "always-skip" wording) so it matches the implementation that bypasses all constraints for secret refs; locate the block containing the comment lines starting with "-- skip_validation: schema-aware hook - secret ref always bypassed..." and replace that heading text with "always-skip hook" and adjust any adjacent descriptive text to reflect the always-skip behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@t/default.lua`:
- Around line 403-406: The test case heading/comment is mislabeled as
"schema-aware hook" but the predicate ignores schema and implements the opposite
behavior; update the test case heading/comment for test case 17 to "always-skip
hook" (or similar "always-skip" wording) so it matches the implementation that
bypasses all constraints for secret refs; locate the block containing the
comment lines starting with "-- skip_validation: schema-aware hook - secret ref
always bypassed..." and replace that heading text with "always-skip hook" and
adjust any adjacent descriptive text to reflect the always-skip behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c3d7cf0e-f910-4681-9c90-72615247abaa
📒 Files selected for processing (2)
lib/jsonschema.luat/default.lua
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/jsonschema.lua
6591904 to
88956af
Compare
Changes since v0.9.12: - feat: add skip_validation custom hook for value-level bypass
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/jsonschema.lua (1)
614-633:⚠️ Potential issue | 🟠 MajorMove
skip_validationbefore the boolean-schema fast paths.Right now the new hook never runs for schemas that resolve to
false, so placeholders in those branches still fail before the bypass can short-circuit. If this is meant to be a value-level escape hatch, the hook needs to run before thetrue/falsereturn path.♻️ Proposed fix
- if schema == true then - ctx:stmt('do return true end') - return ctx - elseif schema == false then - ctx:stmt('do return false, "expect false always" end') - return ctx - end - -- skip_validation hook: if the caller provided a predicate via -- custom.skip_validation, check it before any constraint. When it -- returns true the value is accepted as-is (useful for placeholder @@ if ctx._root._skip_validation then local schema_ref = ctx:uservalue(schema) ctx:stmt(sformat('if %s ~= nil and %s(%s, %s) then return true end', ctx:param(1), ctx:libfunc('custom.skip_validation'), ctx:param(1), schema_ref)) end + + if schema == true then + ctx:stmt('do return true end') + return ctx + elseif schema == false then + ctx:stmt('do return false, "expect false always" end') + return ctx + end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/jsonschema.lua` around lines 614 - 633, The skip_validation hook (ctx._root._skip_validation) must run before the boolean-schema fast paths so it can short-circuit even when schema is literally true/false; move the entire if-block that computes schema_ref and emits the skip_validation check (the block starting with "if ctx._root._skip_validation then" that calls ctx:uservalue(schema) and ctx:libfunc('custom.skip_validation')) to before the initial "if schema == true ... elseif schema == false ..." branch so the predicate runs first and can return true to bypass validation for placeholder values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@lib/jsonschema.lua`:
- Around line 614-633: The skip_validation hook (ctx._root._skip_validation)
must run before the boolean-schema fast paths so it can short-circuit even when
schema is literally true/false; move the entire if-block that computes
schema_ref and emits the skip_validation check (the block starting with "if
ctx._root._skip_validation then" that calls ctx:uservalue(schema) and
ctx:libfunc('custom.skip_validation')) to before the initial "if schema == true
... elseif schema == false ..." branch so the predicate runs first and can
return true to bypass validation for placeholder values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a9eba98a-887e-4ef1-bd30-be46b2e30da4
📒 Files selected for processing (2)
lib/jsonschema.luat/default.lua
Add a new `skip_validation` option to `generate_validator()`. When provided, the generated validator emits a check at the start of each sub-validator: if `custom.skip_validation(value)` returns true, the value is accepted without further constraint checks (type, enum, format, pattern, range, etc.). This is useful for placeholder strings (e.g. secret references like `$secret://vault/key`) that will be resolved at runtime — they need to pass schema validation even when the schema expects a non-string type. The hook is opt-in: when not provided, no extra code is generated and there is zero performance overhead.
88956af to
5650619
Compare
Changes since v0.9.12: - feat: add skip_validation custom hook for value-level bypass
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Changes since v0.9.12: - feat: add skip_validation custom hook for value-level bypass
What
Add a
skip_validationcustom hook to the jsonschema validator. When provided, the hook is called asskip_validation(value, schema)before any constraint checks. If it returnstrue, the value is accepted as-is (bypassing type, enum, format, pattern, range checks etc.).A
value ~= nilguard ensures the hook is never called with nil values.Why
APISIX uses placeholder strings like
$secret://vault/redis/passwordin plugin configs. These strings must pass schema validation even when the field has constraints (enum, pattern, minLength), because they will be resolved to the correct value at runtime.Generated code example
Where
uservalues[2]is the schema node for the current field, allowing the hook to inspectschema.typeand make decisions.Test cases
Added 7 test cases (10-16):