feat: add option to disallow code generation from strings#144
feat: add option to disallow code generation from strings#144anonrig wants to merge 1 commit intofastify:mainfrom
Conversation
anonrig
force-pushed
the
enable-disallow-code-generation-from-strings
branch
from
9f6ec36 to
c69effe
Compare
|
Thanks! can you add documentation for this? |
anonrig
force-pushed
the
enable-disallow-code-generation-from-strings
branch
from
c69effe to
a8f47a6
Compare
I updated the README with the newly added option. Let me know if there's anything missing, and thanks for the review @kibertoad |
|
well this would break fast-json-stringify and all dependents on it. |
I'll open a different pull-request and enable it there. The default value is |
| matrix: | ||
| node-version: ${{ fromJson(inputs.node-versions) }} | ||
| os: [macos-latest, ubuntu-latest, windows-latest] | ||
| disallow-code-generation-from-strings: ${{ inputs.check-disallow-code-generation-from-strings == true && ['true', 'false'] || ['false'] }} |
There was a problem hiding this comment.
Is this the simplest way to express this? Apologies for ignorance but I find this expression hard to follow without a comment, I wonder if other readers if this code feel the same.
|
Dont get me wrong, but I would expect this as a npm script. |
|
|
||
| - name: Run tests | ||
| run: npm test | ||
| run: NODE_OPTIONS="${{ steps.node-flags.outputs.flags }}" npm test |
There was a problem hiding this comment.
I would expect this as a different job, not as an edit of this one.
There was a problem hiding this comment.
I didn't understand. Since it is in a matrix config, it will run 2 different jobs. 1 for false value, and one for true. (or only 1 if it's not enabled)
| | `license-check-allowed-additional` | false | string | | Provide a semicolon separated list of SPDX-license identifiers that you want to additionally allow. | | ||
| | `lint` | false | boolean | `false` | Set to `true` to run the `lint` script in a repository's `package.json`. | | ||
| | `node-versions` | false | string | `'["20", "22"]'` | Provide A JSON array that specifies the Node.js versions on which the job should run. | | ||
| | `check-disallow-code-generation-from-strings` | false | boolean | `false` | Enables --disallow-code-generation-from-strings flag for Node.js | |
There was a problem hiding this comment.
I would prefer to remove the "check-" prefix
|
Is this still something we want? Could this not be an eslint rule? |
I have never understood the need for it. It is stated that it fixes something in Cloudflare Workers, but I don't think we use those to run our CI? In my opinion, if someone is able to make use of these workflows in their environment, they are welcome to do so. But these workflows are not a product unto themselves like any of the things we publish on |
7 participants
evalandnew Functionis not available on platforms such as Cloudflare workers. This option will add support for disabling and testing environments where code generation from strings are not available.