ast: Mark JSON schema builtins nondeterministic - #9100
Open
ARMeeru wants to merge 1 commit into
Open
Conversation
json.match_schema and json.verify_schema dereference `$ref` at evaluation time, including remote and file:// URLs, so their results depend on what those locations serve. Neither was declared Nondeterministic. The visible consequence is in partial evaluation. Because ignoreDuringPartial() keys off that flag, partial eval folded both builtins when their arguments were known, fetching over the network at partial-eval time and baking the answer into the residual policy. With the flag set, the calls are left in the residual and evaluated at query time instead. Network access itself remains gated by the allow_net capability, which already applies to these builtins. capabilities.json is regenerated to carry the new flag. A partial evaluation test covers the retained call; it points at the existing local test server so a regression fails there rather than reaching the network. Fixes: open-policy-agent#8998 Signed-off-by: Asifur Rahaman Meeru <asifur.rahaman@meeru.dev>
ARMeeru
marked this pull request as ready for review
August 29, 2026 05:03
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.
Why the changes in this PR are needed?
json.match_schemaandjson.verify_schemadereference$refat evaluation time, including remote andfile://URLs, so their results depend on what those locations serve. Neither is declaredNondeterministic.Reported in #8998, where the reporter walks the builtin list and disallows nondeterministic builtins when running sandboxed code. These two pass that filter silently.
There is also a concrete consequence in partial evaluation.
ignoreDuringPartial()keys off the same flag, so partial eval folds these calls when their arguments are known, fetching over the network at partial-eval time and baking the answer into the residual policy. A residual built today can encode whatever a remote server happened to serve at build time.What are the changes in this PR?
Nondeterministic: trueon both builtins inv1/ast/builtins.go, andcapabilities.jsonregenerated with./build/gen-run-go.sh. The regenerated file changes exactly those two entries.A partial evaluation case in
v1/topdown/topdown_partial_test.goasserts the call is left in the residual. It fails without the flags, where the residual collapses toinput.x = 1because the call was folded away.CanSkipBctxis unchanged: both builtins need theBuiltinContextto read theallow_netcapability.Notes to assist PR review:
Measured on this branch against
main, same query, with a local HTTP server counting requests:maininput.unrelated = "x", the schema call folded awayjson.match_schemacall retainedOne correction to the issue:
allow_netis not bypassed. Running the same policy withallow_net: []makes zero requests, so remote$reffetching is already gated. Worth stating so this is not read as an unbounded SSRF.The test's
$refpoints at the file's existing local test server rather than a public URL, so if the flags are ever removed the resulting fetch stays on loopback instead of leaving CI.Worth weighing before merge: this changes partial-eval residuals for anyone using these builtins, since calls that were folded are now retained. That is the intended fix, but it is a behaviour change beyond the capabilities metadata.
capabilities/v1.20.0.jsonalready shipped with the old values and is left untouched, so this lands in v1.21.0.Further comments:
Fixes #8998. The reporter confirmed on the issue that marking them plus updating capabilities covers his case, and noted he does not use partial eval directly, so the partial-eval half has not had a maintainer's view yet.
That is the open question I would most like input on: is marking both builtins the change you want, or would you rather treat the partial-eval folding separately?