security: make allow_net restrict file:// refs in JSON schemas by HarnageaGabriel · Pull Request #9044 · open-policy-agent/opa

HarnageaGabriel · 2026-08-18T20:06:12Z

Root cause

jsonReferenceLoader.LoadJSON() in internal/gojsonschema/jsonLoader.go resolved file:// $refs by calling loadFromFile() unconditionally whenever reference.HasFileScheme is true — before the isAllowed() allow-list check is ever consulted:

if reference.HasFileScheme {
    filename := strings.TrimPrefix(refToURL.String(), "file://")
    ...
    return l.loadFromFile(filename)
}
...
if l.isAllowed(refToURL.GetUrl()) {
    return l.loadFromHTTP(refToURL.String())
}

isAllowed() checks ref.Hostname() against limits.allowNet, which is only meaningful for the HTTP branch. allow_net (wired in for json.match_schema/json.verify_schema via newPatternValidatingSchemaLoader in v1/topdown/jsonschema.go) therefore never restricted file:// refs at all — allow_net: [] blocked every remote host but left local file reads on the machine running OPA completely open. Since the schema operand to these builtins can come from input, this allowed input-driven arbitrary local file reads, defeating the sandboxing allow_net is meant to provide.

Fix

Deny file:// refs whenever allow_net is set (including the empty-list case, allow_net: []), consistent with how allow_net already treats every other remote resource: deny by default unless explicitly permitted. When allow_net is unset (nil / unrestricted), file refs keep loading exactly as before — no behavior change for callers who don't configure allow_net.

I considered instead extending the allow-list mechanism to permit specific local paths, but a hostname allow-list doesn't map cleanly onto filesystem paths, so I went with the more conservative deny-outright approach. Flagged this tradeoff on the issue before starting — happy to adjust if maintainers prefer a different shape (e.g. a dedicated capability for local file refs).

Testing

go build ./... passes
go test ./internal/gojsonschema/... ./v1/topdown/... passes (2 pre-existing, unrelated TestCertSelectionLogic failures reproduce identically on unmodified main on this machine — Windows system-cert environment issue, not touched by this change)
Added TestAllowNetRestrictsFileReferences covering: file:// ref denied when allow_net: [], file:// ref still loads when allow_net unset
Existing HTTP allow_net enforcement tests unmodified and still pass

Changelog

Added an entry under Unreleased / Fixes in CHANGELOG.md.

jsonReferenceLoader.LoadJSON resolved file:// $refs by calling loadFromFile unconditionally, before the isAllowed allow-list check. isAllowed only gates the HTTP branch, so allow_net (wired in via newPatternValidatingSchemaLoader for json.match_schema and json.verify_schema) had no effect on file:// refs at all -- even allow_net: [] left local file reads on the machine running OPA completely open. Since the schema operand to those builtins can come from input, this allowed input-driven arbitrary local file reads. Deny file:// refs whenever allow_net is set (including the empty-list case), matching how allow_net already treats every other remote resource: deny by default unless explicitly permitted. When allow_net is unset, file refs keep loading as before. Fixes open-policy-agent#9000 Signed-off-by: Gabriel Harnagea <gabriel.harnagea06@gmail.com>

charlieegan3 · 2026-08-19T09:38:47Z

 }

+func TestAllowNetRestrictsFileReferences(t *testing.T) {
+	filename := filepath.Join(t.TempDir(), "schema.json")


might be able to use https://github.com/open-policy-agent/opa/blob/main/v1/util/test/tempfs.go#L20

charlieegan3 · 2026-08-19T09:52:33Z


+### Fixes
+
+- Security: Make `allow_net` restrict `file://` references in JSON schemas used by


Generally this is just updated during the release process.

charlieegan3 · 2026-08-19T09:55:16Z

+		allowNet   []string
+		wantDenied bool
+	}{
+		{note: "nil list permits file references", allowNet: nil},


Setting allow_net: [], to block file access feels incorrect here.

I think that we ought to default to not allowing file:// URIs in schemas. We might want to add another capability to allow this, that defaults to never allowing local file access.

AllowFileSystemPaths: default [] is one idea.

@charlieegan3

Revert the CHANGELOG.md entry -- per @charlieegan3, that file is updated during the release process, not by individual PRs. Switch the file-reference test to the existing test.TempDirOf helper from v1/util/test instead of manually building the temp file, per @charlieegan3's pointer to that helper. Note: charlieegan3 also suggested decoupling the file:// deny from allow_net entirely (a dedicated capability defaulting to deny, independent of whether allow_net is set). Tried that as a follow-up and confirmed it regresses TestFragmentLoader and TestFileWithSpace, which load local schema files directly (not via an untrusted ) and never set allow_net -- so an unconditional deny breaks legitimate trusted-caller file loading, not just the vulnerable path. Left the allow_net-gated behavior in place pending a properly scoped follow-up that only denies for the json.verify_schema/json.match_schema builtin path specifically. Signed-off-by: Gabriel Harnagea <gabriel.harnagea06@gmail.com>

@charlieegan3

Revert the CHANGELOG.md entry -- per @charlieegan3, that file is updated during the release process, not by individual PRs. Switch the file-reference test to the existing test.TempDirOf helper from v1/util/test instead of manually building the temp file, per @charlieegan3's pointer to that helper. Note: charlieegan3 also suggested decoupling the file:// deny from allow_net entirely (a dedicated capability defaulting to deny, independent of whether allow_net is set). Tried that as a follow-up and confirmed it regresses TestFragmentLoader and TestFileWithSpace, which load local schema files directly (not via an untrusted ) and never set allow_net -- so an unconditional deny breaks legitimate trusted-caller file loading, not just the vulnerable path. Left the allow_net-gated behavior in place pending a properly scoped follow-up that only denies for the json.verify_schema/json.match_schema builtin path specifically.

HarnageaGabriel · 2026-08-19T19:51:56Z

Thanks for the review. Pushed two of the three fixes: reverted the CHANGELOG.md entry, and switched the test to test.TempDirOf.

On the third one (decoupling file:// deny from allow_net into its own capability): I tried it and it regresses TestFragmentLoader/TestFileWithSpace, which load local schema files directly (not via an attacker-controlled $ref) and never set allow_net -- so an unconditional deny breaks legitimate trusted-caller file loading, not just the json.verify_schema/json.match_schema path this PR is actually about. Doing it properly means a capability that's set specifically by newPatternValidatingSchemaLoader (the builtin-facing path) and left unset everywhere else, rather than a global always-deny. That's more surface than I want to add speculatively in this PR -- happy to take it on as a follow-up once the shape of AllowFileSystemPaths (or whatever it ends up being called) is settled, or if you'd rather I attempt it here, let me know.

charlieegan3 · 2026-08-20T09:52:07Z

That's more surface than I want to add speculatively in this PR

Fair enough, but I think if we want to actually address #9000 that is what's needed. I do not think that reusing allow_net is appropriate here.

We need to have two different paths for schema loading for both v1/topdown/jsonschema.go (often untrusted), and v1/ast/compile.go (mostly trusted).

allow_net previously doubled as the gate for file:// $ref access, which conflated remote-fetch policy with local-file policy and forced trusted callers (compile-time schema annotations) and untrusted callers (the json.verify_schema/json.match_schema builtins, where schemas may come from input) to share one on/off switch. Add a dedicated DenyFileScheme flag to gojsonschema.SchemaLoader, independent of AllowNet. newPatternValidatingSchemaLoader (backing the builtins) now sets it unconditionally, so file:// refs are denied regardless of allow_net's value. v1/ast/compile.go's compile-time loader leaves it unset, preserving existing trusted file-loading behavior. AllowNet goes back to governing only remote HTTP fetches. Signed-off-by: Gabriel Harnagea <gabriel.harnagea06@gmail.com>

HarnageaGabriel · 2026-08-24T10:02:25Z

Pushed d7349b1a6 implementing the two-path split you described.

internal/gojsonschema/schemaLoader.go — added a new independent SchemaLoader.DenyFileScheme field (zero value false). AllowNet no longer has any bearing on file:// loading; it only governs remote HTTP fetches again.

internal/gojsonschema/jsonLoader.go — LoadJSON()'s HasFileScheme branch now checks denyFileScheme instead of allowNet != nil. Distinct error message ("file reference loading disabled") so it's not confused with the HTTP-fetch-denial message.

v1/topdown/jsonschema.go — newPatternValidatingSchemaLoader (backs json.verify_schema/json.match_schema) now sets sl.DenyFileScheme = true unconditionally. File refs are denied on this path no matter what allow_net is set to, since schemas reaching these builtins can come from input.

v1/ast/compile.go — untouched. compileSchema never sets DenyFileScheme, so compile-time # schema annotation loading keeps working exactly as before, unaffected by this change.

Tests:

internal/gojsonschema/jsonschema_test.go: replaced the old TestAllowNetRestrictsFileReferences with TestFileReferencesIgnoreAllowNet (proves AllowNet no longer gates file access) and TestDenyFileSchemeRestrictsFileReferences (proves the new flag denies file refs regardless of AllowNet's value).
v1/topdown/jsonschema_test.go: added TestBuiltinJSONSchemaDeniesFileReferences, exercising both json.verify_schema and json.match_schema with nil, unset, empty, and populated allow_net capabilities — all four deny the file reference, directly verifying the security property from allow_net does not restrict file:// $refs in schema built-ins #9000 at the builtin level.
internal/gojsonschema/schema_test.go's TestFragmentLoader/TestFileWithSpace (trusted local-file loading via NewReferenceLoader, not through SchemaLoader) are unaffected and still pass.

Full runs: go test ./internal/gojsonschema/... and the schema-relevant parts of ./v1/topdown/... / ./v1/ast/... all green. (Ran on Windows locally — hit a couple of pre-existing, unrelated platform-specific test failures around Unix path assumptions and a Go 1.26 zero-duration timer assertion; confirmed both fail identically on the base commit without this change, so unrelated to this PR.)

charlieegan3 · 2026-08-25T12:34:26Z

withRemoteRefLimits and remoteRefLimits are private, so we might want to consider a rename to something more generic like loaderLimits or loadPolicy since file system access is not actually "remote".

charlieegan3 · 2026-08-25T12:34:46Z


+	// A file:// $ref is denied whenever DenyFileScheme is set, independent
+	// of allowNet. allowNet governs only remote HTTP fetches.
+	denyFileScheme bool


I might be inclined to call this allowFilesystem to match the polarity of allowNet, just make it default false.

charlieegan3 · 2026-08-25T12:35:32Z

Hi there, two small comments. Please also make sure that DCO is passing, this needs to be your email in the commit messages.

@charlieegan3

Per @charlieegan3's naming suggestions on PR open-policy-agent#9044: - remoteRefLimits/withRemoteRefLimits -> loaderLimits/withLoaderLimits, since filesystem access was never actually "remote". - DenyFileScheme -> AllowFilesystem (default false), matching AllowNet's polarity. Trusted callers (compile.go's compileSchema, schema.go's NewSchema) now explicitly set AllowFilesystem: true; the builtin-facing path (topdown/jsonschema.go) leaves it at its deny-by-default zero value instead of explicitly setting a deny flag. Signed-off-by: Gabriel Harnagea <gabriel.harnagea06@gmail.com>

HarnageaGabriel · 2026-08-30T21:23:30Z

Fixed DCO (two commits were missing sign-off) and applied both naming suggestions: remoteRefLimits/withRemoteRefLimits -> loaderLimits/withLoaderLimits, and DenyFileScheme -> AllowFilesystem (default false, matching allowNet's polarity). Trusted callers (compileSchema, NewSchema) now explicitly opt in with AllowFilesystem: true; the builtin-facing path just leaves it at the deny-by-default zero value. CI and DCO are green.

charlieegan3 · 2026-09-01T11:05:38Z

+	}{
+		{note: "nil list", allowNet: nil},
+		{note: "empty list", allowNet: []string{}},
+		{note: "populated list", allowNet: []string{"example.com"}},


I'd not really expect to see these tests based on allowNet now we are using a new limit setting.

charlieegan3 · 2026-09-01T11:06:28Z

Hi thanks for coming back to this, can you have a look at the above comment and rebase this? I think we just need to make sure the tests are updated too and testing the correct limit option, not allowNet.

charlieegan3 · 2026-09-01T11:07:20Z

+	// Local file access is allowed only when allowFilesystem is set, independent
+	// of allowNet. allowNet governs only remote HTTP fetches.


Suggested change

// Local file access is allowed only when allowFilesystem is set, independent

// of allowNet. allowNet governs only remote HTTP fetches.

// Local file access is allowed only when allowFilesystem is set.

Allownet is 'unrelated' as far as the field here is listed.

charlieegan3 · 2026-09-01T11:09:42Z

+			capabilities: &ast.Capabilities{},
+		},
+		{
+			note:         "empty allow_net",


Again, this is strange as it's allow_net, but then a file schema ref in the JSON above.

charlieegan3 reviewed Aug 19, 2026

View reviewed changes

charlieegan3 reviewed Aug 25, 2026

View reviewed changes

HarnageaGabriel force-pushed the fix-issue-9000-file-ref-allow-net branch from d7349b1 to 136fad1 Compare August 26, 2026 14:55

charlieegan3 reviewed Sep 1, 2026

View reviewed changes


		### Fixes

		- Security: Make `allow_net` restrict `file://` references in JSON schemas used by

		// Local file access is allowed only when allowFilesystem is set, independent
		// of allowNet. allowNet governs only remote HTTP fetches.

Uh oh!

Conversation

HarnageaGabriel commented Aug 18, 2026

Root cause

Fix

Testing

Changelog

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HarnageaGabriel commented Aug 19, 2026

Uh oh!

charlieegan3 commented Aug 20, 2026

Uh oh!

HarnageaGabriel commented Aug 24, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charlieegan3 commented Aug 25, 2026

Uh oh!

HarnageaGabriel commented Aug 30, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charlieegan3 commented Sep 1, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants