ast: Enforce inlined schemas without --schemas flag (#5689)

* ast: Enforce inlined schemas without --schemas flag

Enabled for commands:
* eval
* check
* test

Always parse annotations.
Always use `schemas` annotations with inlined schemas for type checking.
Ignore `schemas` annotations with schema refs if no `--schema` flag was provided.

Fixes: #5506

Signed-off-by: Johan Fylling <johan.dev@fylling.se>

ast/check.go

@@ -184,6 +184,9 @@ func (tc *typeChecker) checkRule(env *TypeEnv, as *AnnotationSet, rule *Rule) {
 				tc.err([]*Error{err})
 				continue
 			}
+			if ref == nil && refType == nil {
+				continue
+			}
 			prefixRef, t := getPrefix(env, ref)
 			if t == nil || len(prefixRef) == len(ref) {
 				env.tree.Put(ref, refType)
@@ -1213,6 +1216,9 @@ func processAnnotation(ss *SchemaSet, annot *SchemaAnnotation, rule *Rule, allow
 	var schema interface{}
 
 	if annot.Schema != nil {
+		if ss == nil {
+			return nil, nil, nil
+		}
 		schema = ss.Get(annot.Schema)
 		if schema == nil {
 			return nil, nil, NewError(TypeErr, rule.Location, "undefined schema: %v", annot.Schema)

cmd/check.go

@@ -65,7 +65,7 @@ func checkModules(params checkParams, args []string) error {
 		for _, path := range args {
 			b, err := loader.NewFileLoader().
 				WithSkipBundleVerification(true).
-				WithProcessAnnotation(ss != nil).
+				WithProcessAnnotation(true).
 				WithCapabilities(capabilities).
 				AsBundle(path)
 			if err != nil {
@@ -81,7 +81,7 @@ func checkModules(params checkParams, args []string) error {
 		}
 
 		result, err := loader.NewFileLoader().
-			WithProcessAnnotation(ss != nil).
+			WithProcessAnnotation(true).
 			WithCapabilities(capabilities).
 			Filtered(args, f.Apply)
 		if err != nil {
@@ -99,7 +99,7 @@ func checkModules(params checkParams, args []string) error {
 		WithSchemas(ss).
 		WithEnablePrintStatements(true).
 		WithStrict(params.strict).
-		WithUseTypeCheckAnnotations(ss != nil)
+		WithUseTypeCheckAnnotations(true)
 
 	compiler.Compile(modules)
 	if compiler.Failed() {

cmd/check_test.go

@@ -120,3 +120,52 @@ p if "opa" in input.tools`,
 		})
 	}
 }
+
+func testCheckWithSchemasAnnotationButNoSchemaFlag(policy string) error {
+	files := map[string]string{
+		"test.rego": policy,
+	}
+
+	var err error
+	test.WithTempFS(files, func(path string) {
+		params := newCheckParams()
+
+		err = checkModules(params, []string{path})
+	})
+
+	return err
+}
+
+// Assert that 'schemas' annotations with schema refs are only informing the type checker when the --schema flag is used
+func TestCheckWithSchemasAnnotationButNoSchemaFlag(t *testing.T) {
+	policyWithSchemaRef := `
+package test
+# METADATA
+# schemas:
+#   - input: schema["input"]
+p { 
+	rego.metadata.rule() # presence of rego.metadata.* calls must not trigger unwanted schema evaluation 
+	input.foo == 42 # type mismatch with schema that should be ignored
+}`
+
+	err := testCheckWithSchemasAnnotationButNoSchemaFlag(policyWithSchemaRef)
+	if err != nil {
+		t.Fatalf("unexpected error from eval with schema ref: %v", err)
+	}
+
+	policyWithInlinedSchema := `
+package test
+# METADATA
+# schemas:
+#   - input.foo: {"type": "boolean"}
+p { 
+	rego.metadata.rule() # presence of rego.metadata.* calls must not trigger unwanted schema evaluation 
+	input.foo == 42 # type mismatch with schema that should be ignored
+}`
+
+	err = testCheckWithSchemasAnnotationButNoSchemaFlag(policyWithInlinedSchema)
+	// We expect an error here, as inlined schemas are always used for type checking
+	if !strings.Contains(err.Error(), "rego_type_error: match error") {
+		t.Fatalf("unexpected error from eval with inlined schema, got: %v", err)
+	}
+}

cmd/eval_test.go

@@ -15,6 +15,7 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
+	"strings"
 	"testing"
 
 	"github.com/open-policy-agent/opa/ast"
@@ -470,14 +471,14 @@ func testEvalWithSchemasAnnotationButNoSchemaFlag(policy string) error {
 		var defined bool
 		defined, err = eval([]string{query}, params, &buf)
 		if !defined || err != nil {
-			err = fmt.Errorf("Unexpected error or undefined from evaluation: %v", err)
+			err = fmt.Errorf(buf.String())
 		}
 	})
 
 	return err
 }
 
-// Assert that 'schemas' annotations are only informing the type checker when the --schema flag is used
+// Assert that 'schemas' annotations with schema refs are only informing the type checker when the --schema flag is used
 func TestEvalWithSchemasAnnotationButNoSchemaFlag(t *testing.T) {
 	policyWithSchemaRef := `
 package test
@@ -505,8 +506,9 @@ p {
 }`
 
 	err = testEvalWithSchemasAnnotationButNoSchemaFlag(policyWithInlinedSchema)
-	if err != nil {
-		t.Fatalf("unexpected error from eval with inlined schema: %v", err)
+	// We expect an error here, as inlined schemas are always used for type checking
+	if !strings.Contains(err.Error(), `"code": "rego_type_error"`) {
+		t.Fatalf("unexpected error from eval with inlined schema, got: %v", err)
 	}
 }
 

cmd/test_test.go

@@ -3,8 +3,10 @@ package cmd
 import (
 	"bytes"
 	"context"
+	"strings"
 	"testing"
 
+	"github.com/open-policy-agent/opa/ast"
 	"github.com/open-policy-agent/opa/rego"
 	"github.com/open-policy-agent/opa/topdown"
 	"github.com/open-policy-agent/opa/util/test"
@@ -228,7 +230,7 @@ func testSchemasAnnotation(mod string) error {
 	return err
 }
 
-// Assert that 'schemas' annotations are ignored
+// Assert that 'schemas' annotations with schema ref are ignored, but not inlined schemas
 func TestSchemasAnnotation(t *testing.T) {
 	policyWithSchemaRef := `
 package test
@@ -264,7 +266,9 @@ test_p {
 }`
 
 	err = testSchemasAnnotation(policyWithInlinedSchema)
-	if err != nil {
-		t.Fatalf("unexpected error when inlined schema is present: %v", err)
+	if err == nil {
+		t.Fatalf("didn't get expected error when inlined schema is present")
+	} else if !strings.Contains(err.Error(), "rego_type_error: match error") {
+		t.Fatalf("didn't get expected %s error when inlined schema is present; got: %v", ast.TypeErr, err)
 	}
 }

docs/content/annotations.md

@@ -225,9 +225,13 @@ allow {
 The `schemas` annotation is a list of key value pairs, associating schemas to data values.
 In-depth information on this topic can be found [here](../schemas#schema-annotations).
 
-#### Example
+#### Schema Reference Format
 
-```live:rego/metadata/schemas:module:read_only
+Schema files can be referenced by path, where each path starts with the `schema` namespace, and trailing components specify 
+the path of the schema file (sans file-ending) relative to the root directory specified by the `--schema` flag on applicable commands.
+If the `--schema` flag is not present, referenced schemas are ignored during type checking.
+
+```live:rego/metadata/schemas_ref:module:read_only
 # METADATA
 # schemas:
 #   - input: schema.input
@@ -238,6 +242,21 @@ allow {
 }
 ```
 
+#### Inlined Schema Format
+
+Schema definitions can be inlined by specifying the schema structure as a YAML or JSON map.
+Inlined schemas are always used to inform type checking for the `eval`, `check`, and `test` commands; 
+in contrast to [by-reference schema annotations](#schema-reference-format), which require the `--schema` flag to be present in order to be evaluated.
+
+```live:rego/metadata/schemas_inline:module:read_only
+# METADATA
+# schemas:
+#   - input.x: {type: number}
+allow {
+    input.x == 42
+}
+```
+
 ### Entrypoint
 
 The `entrypoint` annotation is a boolean used to mark rules and packages that should be used as entrypoints for a policy.

rego/rego.go

@@ -1156,7 +1156,7 @@ func New(options ...func(r *Rego)) *Rego {
 			WithCapabilities(r.capabilities).
 			WithEnablePrintStatements(r.enablePrintStatements).
 			WithStrict(r.strict).
-			WithUseTypeCheckAnnotations(r.schemaSet != nil)
+			WithUseTypeCheckAnnotations(true)
 	}
 
 	if r.store == nil {
@@ -1753,7 +1753,7 @@ func (r *Rego) loadFiles(ctx context.Context, txn storage.Transaction, m metrics
 
 	result, err := loader.NewFileLoader().
 		WithMetrics(m).
-		WithProcessAnnotation(r.schemaSet != nil).
+		WithProcessAnnotation(true).
 		Filtered(r.loadPaths.paths, r.loadPaths.filter)
 	if err != nil {
 		return err
@@ -1782,7 +1782,7 @@ func (r *Rego) loadBundles(ctx context.Context, txn storage.Transaction, m metri
 	for _, path := range r.bundlePaths {
 		bndl, err := loader.NewFileLoader().
 			WithMetrics(m).
-			WithProcessAnnotation(r.schemaSet != nil).
+			WithProcessAnnotation(true).
 			WithSkipBundleVerification(r.skipBundleVerification).
 			AsBundle(path)
 		if err != nil {