cmd/cue: define UX for validating JSON with JSON Schema that includes "additionalProperties": false

[cueckoo]

trafficstars

Originally opened by @breml in https://github.com/cuelang/cue/issues/750

What version of CUE are you using (cue version)?

$ cue version
cue version 0.3.0-beta.4 linux/amd64

Does this issue reproduce with the latest release?

Yes

What did you do?

I have an existing JSON Schema (schema.json), that includes "additionalProperties": false like this:

{
  "$schema": "http://json-schema.org/draft-07/schema#",
  "title": "test",
  "type": "object",
  "required": ["mandatory"],
  "additionalProperties": false,
  "properties": {
    "mandatory": {
      "description": "mandatory field",
      "type": "string"
    },
    "optional": {
      "description": "optional field",
      "type": "string"
    }
  }
}

I imported this with cue:

$ cue import schema.json

Then I tried to validate a JSON file (data.json) with the following content

{
  "mandatory": "foo",
  "optional": "bar",
  "invalid": true
}

with

$ cue vet data.json schema.cue

What did you expect to see?

I expect this command to fail, because the JSON file does include an additional field (invalid) and this is not allowed with "additionalProperties": false.

What did you see instead?

cue did not complain.

[cueckoo]

Original reply by @breml in https://github.com/cuelang/cue/issues/750#issuecomment-776484838

Might be related to #267.

[cueckoo]

Original reply by @myitcv in https://github.com/cuelang/cue/issues/750#issuecomment-776549247

The issue here is that by default cue import by default imports the schema as follows:

// schema.cue
// test
@jsonschema(schema="http://json-schema.org/draft-07/schema#")
// mandatory field
mandatory: string
// optional field
optional?: string

Then via cue vet the root of the loaded CUE files (i.e. the value {mandatory: string, optional?: string}) is used to validate data.json.

Notice however the root value is not closed. Hence, regardless of whether additionalProperties is supplied or not, additional fields will be allowed.

@mpvl and I discussed this in the context of imports of package values the other day. This is another interesting data point in the question of whether package values should be closed or not.

On a related note the following also "fails":

cue vet jsonschema: schema.json json: data.json

I suspect for the same reason. However from a user's perspective, this workflow should probably work without any further effort (independent of how/whether we address the package closedness question)

[cueckoo]

Original reply by @mpvl in https://github.com/cuelang/cue/issues/750#issuecomment-780075105

@myitcv: there is more going on here, in fact.

The conversion also doesn't close structs that are not at the root. The reason is that closeness in CUE means something different from closeness in JSON schema (additionalProperties: false).

The purpose of closeness in CUE is 1) represent proto-like oneofs, 2) catch typos, or basically validation within a single version. The belief is though, that an API should not close the door for future expansion and has no means of representing this. So closeness it to detect types in a closed environment, but unknown fields of incoming messages should be ignored. A very close analogy is how Proto definitions are closed when generated for, say, a Go program, but that unknown fields in a received message within that same program are ignored.

It is quite unclear to me how JSON Schema should be interpreted in this context. There seems to be about as little guidance around it and it seems about as vague as the concept of null in JSON. There is a reason its use is restricted in structural form OpenAPI for CRDs.

But open to suggestions on how this should be translated. There seems to be at least something amiss. If there is an additionalProperties: false in the input schema, at the very least the schema in CUE should be closed. But arguably it should always be closed regardless. Or it should be a command line option. See also discussion in #267.

[jpluscplusm]

If there is an additionalProperties: false in the input schema, at the very least the schema in CUE should be closed.

This has just bitten me. Ok - not "bitten", but "surprised" me :-)

Importing a jsonschema with an explicit additionalProperties:false at any level, including the root, still produces a CUE struct that's deliberately left open.

I /think/ that the above quote is true, and that additionalProperties:false should close the struct it's inside. I can't see that this has anything to do with API-evolution-over-time, as it's an explicit request from the schema to disallow any unmentioned keys.

[myitcv]

Dropping the now-defunct v0.4.x milestone from this issue, but leaving the zGarden label such that we come round to considering what milestone this should sit in.