Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid JSON Schema enforcement of additionalProperties #22349

Closed
1 of 4 tasks
JacobLey opened this issue Mar 17, 2024 · 14 comments · Fixed by #22426
Closed
1 of 4 tasks

Invalid JSON Schema enforcement of additionalProperties #22349

JacobLey opened this issue Mar 17, 2024 · 14 comments · Fixed by #22426
Labels

Comments

@JacobLey
Copy link
Contributor

Current Behavior

A executor schema that allows an object with arbitrary keys so long as the values match.

Typescript equivalent: Record<{ foo: Record<string, number> }>

{
  "type": "object",
  "additionalProperties": false,
  "properties": {
    "foo": {
      "type": "object",
      "additionalProperties": {
          "type": "number"
      }
    }
  }
}

Running the executor throws with



 NX   Cannot convert undefined or null to object


TypeError: Cannot convert undefined or null to object
    at Function.keys (<anonymous>)
    at /workspace/node_modules/.pnpm/[email protected]/node_modules/nx/src/utils/params.js:194:24
    at Array.find (<anonymous>)
    at validateObject (/workspace/node_modules/.pnpm/[email protected]/node_modules/nx/src/utils/params.js:193:27)
    at validateProperty (/workspace/node_modules/.pnpm/[email protected]/node_modules/nx/src/utils/params.js:346:9)
    at /workspace/node_modules/.pnpm/[email protected]/node_modules/nx/src/utils/params.js:210:9
    at Array.forEach (<anonymous>)
    at validateObject (/workspace/node_modules/.pnpm/[email protected]/node_modules/nx/src/utils/params.js:209:23)
    at validateOptsAgainstSchema (/workspace/node_modules/.pnpm/[email protected]/node_modules/nx/src/utils/params.js:144:5)
    at combineOptionsForExecutor (/workspace/node_modules/.pnpm/[email protected]/node_modules/nx/src/utils/params.js:432:5)

Expected Behavior

Executor can run successfully. Schema is treated as valid.

GitHub Repo

No response

Steps to Reproduce

  1. Create an executor schema that has a "type": "object" with "additionalProperties" set to anything but true.
  2. Attempt to run the executor

Nx Report

Node   : 20.11.1
OS     : linux-arm64
pnpm   : 8.15.4

nx (global)  : 18.1.1
nx           : 18.1.1
@nx/eslint   : 18.1.1
@nrwl/tao    : 18.1.1

Failure Logs

NX   Cannot convert undefined or null to object


TypeError: Cannot convert undefined or null to object
    at Function.keys (<anonymous>)
    at /workspace/node_modules/.pnpm/[email protected]/node_modules/nx/src/utils/params.js:194:24
    at Array.find (<anonymous>)
    at validateObject (/workspace/node_modules/.pnpm/[email protected]/node_modules/nx/src/utils/params.js:193:27)
    at validateProperty (/workspace/node_modules/.pnpm/[email protected]/node_modules/nx/src/utils/params.js:346:9)
    at /workspace/node_modules/.pnpm/[email protected]/node_modules/nx/src/utils/params.js:210:9
    at Array.forEach (<anonymous>)
    at validateObject (/workspace/node_modules/.pnpm/[email protected]/node_modules/nx/src/utils/params.js:209:23)
    at validateOptsAgainstSchema (/workspace/node_modules/.pnpm/[email protected]/node_modules/nx/src/utils/params.js:144:5)
    at combineOptionsForExecutor (/workspace/node_modules/.pnpm/[email protected]/node_modules/nx/src/utils/params.js:432:5)

Package Manager Version

pnpm 8.15.4

Operating System

  • macOS
  • Linux
  • Windows
  • Other (Please specify)

Additional Information

Offending logic is right where the error logs point us: https://github.com/nrwl/nx/blob/master/packages/nx/src/utils/params.ts#L293

If additionalProperties is found, the logic attempts to get the keys of properties. However there is no guarantee that value exists, and Object.keys(undefined) is an error.

The JSONSchema spec does not specify that properties must be set in order to use additionalProperties (as a general rule of thumb, no fields allow/disallow others).

This logic should default properties to {} in the case where it does not exist.

Alternatively, I recommend switching to a battle tested JSON Schema validator, such as AJV

@ivibe
Copy link

ivibe commented Mar 17, 2024

I believe this affects linting. The library or application with project.json without

"targets": {
  ...
  "lint": {
    "executor": "@nx/eslint:lint"
  },
  ...
}

will trigger this error when nx run lib-name:lint --verbose

NX   Cannot convert undefined or null to object


TypeError: Cannot convert undefined or null to object

@robertIsaac
Copy link
Contributor

robertIsaac commented Mar 17, 2024

I believe this affects linting. The library or application with project.json without

"targets": {
  ...
  "lint": {
    "executor": "@nx/eslint:lint"
  },
  ...
}

will trigger this error when nx run lib-name:lint --verbose

NX   Cannot convert undefined or null to object


TypeError: Cannot convert undefined or null to object

yes it fails for me as well
I tried to debug it
the schema is this object

{
  type: 'object',
  description: 'Environment variables that will be made available to the commands. This property has priority over the `.env` files.',
  additionalProperties: { type: 'string' }
}

in this line https://github.com/nrwl/nx/blob/master/packages/nx/src/utils/params.ts#L293
replacing it with
if (Object.keys(schema.additionalProperties).indexOf(p) === -1 &&
make the error go away

hakalb added a commit to codeware-sthlm/nx-plugins that referenced this issue Mar 17, 2024
github-merge-queue bot pushed a commit to codeware-sthlm/nx-plugins that referenced this issue Mar 17, 2024
@mattfysh
Copy link

mattfysh commented Mar 18, 2024

This breaks any usage of nx:run-commands where the env option is defined:

https://github.com/nrwl/nx/blob/master/packages/nx/src/executors/run-commands/schema.json#L121-L126

@robertIsaac I tried your workaround, the error no longer appeared but the environment variables were not present for the running command

@robinsjovoll
Copy link

This also breaks the usage of the new define options for esbuild in Angular 17.2

@grumpyoldman-io
Copy link

grumpyoldman-io commented Mar 18, 2024

Running in to the same error as @mattfysh, holistically it surprises me that there is an additionalProperties field on the schema and not a properties field. I think the bug creeped in because of that reason (someone assuming a properties field would be present when additionalProperties are).

Reverting to [email protected] unblocked me - bug introduced here: 4b88f48#diff-55a4c7f03251ea53a875e8157a13c738f318af858ce94f22c44895fa36019602R290

@c4spar
Copy link

c4spar commented Mar 20, 2024

Running into the same issue with the headers option from the dev-server.

The issue exists since 18.1.0-beta.1

{
  // ...
  "serve": {
    "executor": "@angular-devkit/build-angular:dev-server",
    "options": {
      // ...
      "headers": {
        "foo": "bar"
      }
    }
  }
}
$ pnpm nx serve <app> --verbose
TypeError: Cannot convert undefined or null to object
    at Function.keys (<anonymous>)
    at node_modules/nx/src/utils/params.js:194:24
    at Array.find (<anonymous>)
    at validateObject (node_modules/nx/src/utils/params.js:193:27)
    at validateProperty (node_modules/nx/src/utils/params.js:346:9)
    at node_modules/nx/src/utils/params.js:210:9
    at Array.forEach (<anonymous>)
    at validateObject (node_modules/nx/src/utils/params.js:209:23)
    at validateOptsAgainstSchema (node_modules/nx/src/utils/params.js:144:5)
    at combineOptionsForExecutor (node_modules/nx/src/utils/params.js:432:5)

@noelsoong
Copy link

same issue here

TypeError: Cannot convert undefined or null to object
    at Function.keys (<anonymous>)
    at /Users/noelsoong/Workspace/iticket-stack/node_modules/.pnpm/[email protected]/node_modules/nx/src/utils/params.js:194:24
    at Array.find (<anonymous>)
    at validateObject (/Users/noelsoong/Workspace/iticket-stack/node_modules/.pnpm/[email protected]/node_modules/nx/src/utils/params.js:193:27)
    at validateProperty (/Users/noelsoong/Workspace/iticket-stack/node_modules/.pnpm/[email protected]/node_modules/nx/src/utils/params.js:346:9)
    at /Users/noelsoong/Workspace/iticket-stack/node_modules/.pnpm/[email protected]/node_modules/nx/src/utils/params.js:210:9
    at Array.forEach (<anonymous>)
    at validateObject (/Users/noelsoong/Workspace/iticket-stack/node_modules/.pnpm/[email protected]/node_modules/nx/src/utils/params.js:209:23)
    at validateOptsAgainstSchema (/Users/noelsoong/Workspace/iticket-stack/node_modules/.pnpm/[email protected]/node_modules/nx/src/utils/params.js:144:5)
    at combineOptionsForExecutor (/Users/noelsoong/Workspace/iticket-stack/node_modules/.pnpm/[email protected]/node_modules/nx/src/utils/params.js:432:5)

@mattfysh
Copy link

Anyone who is running into this error, were you able to track down which option was causing it? For me it was the env option of the nx:run-commands executor

If you apply the fix (see #22426 - use patch-package, pnpm patch, or edit the node_modules folder directly), does the error go away? If yes, is the option that triggered the error being applied or ignored?

In the case of env, after applying the fix the error goes away but the option is not applied (ie. environment variables still not accessible in the command/commands)

@jbadeau
Copy link

jbadeau commented Mar 26, 2024

Also an issue with our internal plugins. Any ETA?

@robertIsaac
Copy link
Contributor

Anyone who is running into this error, were you able to track down which option was causing it? For me it was the env option of the nx:run-commands executor

If you apply the fix (see #22426 - use patch-package, pnpm patch, or edit the node_modules folder directly), does the error go away? If yes, is the option that triggered the error being applied or ignored?

In the case of env, after applying the fix the error goes away but the option is not applied (ie. environment variables still not accessible in the command/commands)

I actually have no configuration at all
that's the only part related to lint in the whole project

  "plugins": [
    {
      "plugin": "@nx/eslint/plugin",
      "options": {
        "targetName": "lint"
      }
    },
   ...

@mattfysh
Copy link

@robertIsaac in the case of plugins, they are generating configuration for you

you can see this by running nx show project xyz --web to see the configuration that is generated by the plugin

@jon9090
Copy link

jon9090 commented Mar 27, 2024

I wish to include my custom flag (-t example) when executing the nx angular serve command.

However, I encounter an error from nx indicating:

't' is not found in schema

I intend to intercept this flag within proxy.mjs and handle it accordingly.

What steps should I take to pass this argument without encountering an error?

@robertIsaac
Copy link
Contributor

@robertIsaac in the case of plugins, they are generating configuration for you

you can see this by running nx show project xyz --web to see the configuration that is generated by the plugin

aha I see

{
  "cwd": "apps/fe",
  "env": {
    "ESLINT_USE_FLAT_CONFIG": "true"
  }
}

so env was the cause

Copy link

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.