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

[chore]: port to official @iobroker/eslint-config config #2898

Merged
merged 16 commits into from
Sep 23, 2024

Conversation

foxriver76
Copy link
Collaborator

Implementation details

We now have a centralized eslint config at @iobroker/eslint-config
Adding it to controller was a first step for checking the required rules, there might be changes later on but if they differ to much we will make a separate export for controller and adapters.

Some rules are a bit stricter than before but it now also uses more TS knowledge and can e.g. auto remove unnecessary non-null assertions, unnecessary type casts etc.

/* eslint-disable @typescript-eslint/no-unused-vars */

import { maybeCallback, maybeCallbackWithError, maybeCallbackWithRedisError } from '@/lib/common/maybeCallback.js';

// ============================================================
// maybeCallbackWithError => Callback

async () => {
() => {

Check warning

Code scanning / CodeQL

Expression has no effect Warning

This expression has no effect.
@@ -29,7 +28,7 @@
maybeCallbackWithError(cb, 'this is an error', 'why', 'are', 'you', 'calling', 'me', 'with', 'arguments');
};

async () => {
() => {

Check warning

Code scanning / CodeQL

Expression has no effect Warning

This expression has no effect.
@@ -52,14 +51,14 @@
maybeCallbackWithError(cb, 'this is an error', 'why', 'are', 'you', 'calling', 'me', 'with', 'arguments');
};

async () => {
() => {

Check warning

Code scanning / CodeQL

Expression has no effect Warning

This expression has no effect.
// maybeCallbackWithError, callback only takes an argument that's not compatible with Error
const cb = (err?: number): void => {};
// @ts-expect-error
maybeCallbackWithError(cb, null);
};

async () => {
() => {

Check warning

Code scanning / CodeQL

Expression has no effect Warning

This expression has no effect.
@@ -96,7 +95,7 @@
// ============================================================
// maybeCallbackWithError => Promise

async () => {
() => {

Check warning

Code scanning / CodeQL

Expression has no effect Warning

This expression has no effect.
@@ -199,14 +198,14 @@
maybeCallbackWithRedisError(cb, 'this is an error', 'why', 'are', 'you', 'calling', 'me', 'with', 'arguments');
};

async () => {
() => {

Check warning

Code scanning / CodeQL

Expression has no effect Warning

This expression has no effect.
// maybeCallbackWithRedisError, callback only takes an argument that's not compatible with Error
const cb = (err?: number): void => {};
// @ts-expect-error
maybeCallbackWithRedisError(cb, null);
};

async () => {
() => {

Check warning

Code scanning / CodeQL

Expression has no effect Warning

This expression has no effect.
@@ -243,7 +242,7 @@
// ============================================================
// maybeCallback => Callback

async () => {
() => {

Check warning

Code scanning / CodeQL

Expression has no effect Warning

This expression has no effect.
@@ -255,7 +254,7 @@
maybeCallback(cb, 'why', 'are', 'you', 'calling', 'me', 'with', 'arguments');
};

async () => {
() => {

Check warning

Code scanning / CodeQL

Expression has no effect Warning

This expression has no effect.
@@ -277,7 +276,7 @@
// ============================================================
// maybeCallback => Promise

async () => {
() => {

Check warning

Code scanning / CodeQL

Expression has no effect Warning

This expression has no effect.
@@ -1286,13 +1286,13 @@
async _enumerateAdapterStateObjects(knownObjIDs: string[], adapter: string, instance?: number): Promise<void> {
const adapterRegex = new RegExp(`^${adapter}${instance !== undefined ? `\\.${instance}` : ''}\\.`);
const sysAdapterRegex = new RegExp(
`^system\\.adapter\\.${adapter}${instance !== undefined ? `\\.${instance}` : ''}\\.`
`^system\\.adapter\\.${adapter}${instance !== undefined ? `\\.${instance}` : ''}\\.`,

Check failure

Code scanning / CodeQL

Regular expression injection High

This regular expression is constructed from a
command-line argument
.
@@ -1354,7 +1354,7 @@
private async _enumerateAdapterDocs(knownObjIDs: string[], adapter: string, instance?: number): Promise<void> {
const adapterRegex = new RegExp(`^${adapter}${instance !== undefined ? `\\.${instance}` : ''}\\.`);
const sysAdapterRegex = new RegExp(
`^system\\.adapter\\.${adapter}${instance !== undefined ? `\\.${instance}` : ''}\\.`
`^system\\.adapter\\.${adapter}${instance !== undefined ? `\\.${instance}` : ''}\\.`,

Check failure

Code scanning / CodeQL

Regular expression injection High

This regular expression is constructed from a
command-line argument
.

Copilot Autofix AI 6 months ago

To fix the problem, we need to sanitize the adapter variable before using it to construct the regular expression. The best way to do this is by using the _.escapeRegExp function from the lodash library, which escapes special characters in the string, making it safe to use in a regular expression.

  1. Import the lodash library.
  2. Use the _.escapeRegExp function to sanitize the adapter variable before constructing the regular expression.
Suggested changeset 2
packages/cli/src/lib/setup/setupInstall.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/cli/src/lib/setup/setupInstall.ts b/packages/cli/src/lib/setup/setupInstall.ts
--- a/packages/cli/src/lib/setup/setupInstall.ts
+++ b/packages/cli/src/lib/setup/setupInstall.ts
@@ -26,2 +26,3 @@
 import { createRequire } from 'node:module';
+import _ from 'lodash';
 
@@ -1348,5 +1349,6 @@
     private async _enumerateAdapterDocs(knownObjIDs: string[], adapter: string, instance?: number): Promise<void> {
-        const adapterRegex = new RegExp(`^${adapter}${instance !== undefined ? `\\.${instance}` : ''}\\.`);
+        const safeAdapter = _.escapeRegExp(adapter);
+        const adapterRegex = new RegExp(`^${safeAdapter}${instance !== undefined ? `\\.${instance}` : ''}\\.`);
         const sysAdapterRegex = new RegExp(
-            `^system\\.adapter\\.${adapter}${instance !== undefined ? `\\.${instance}` : ''}\\.`,
+            `^system\\.adapter\\.${safeAdapter}${instance !== undefined ? `\\.${instance}` : ''}\\.`,
         );
EOF
@@ -26,2 +26,3 @@
import { createRequire } from 'node:module';
import _ from 'lodash';

@@ -1348,5 +1349,6 @@
private async _enumerateAdapterDocs(knownObjIDs: string[], adapter: string, instance?: number): Promise<void> {
const adapterRegex = new RegExp(`^${adapter}${instance !== undefined ? `\\.${instance}` : ''}\\.`);
const safeAdapter = _.escapeRegExp(adapter);
const adapterRegex = new RegExp(`^${safeAdapter}${instance !== undefined ? `\\.${instance}` : ''}\\.`);
const sysAdapterRegex = new RegExp(
`^system\\.adapter\\.${adapter}${instance !== undefined ? `\\.${instance}` : ''}\\.`,
`^system\\.adapter\\.${safeAdapter}${instance !== undefined ? `\\.${instance}` : ''}\\.`,
);
packages/cli/package.json
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/cli/package.json b/packages/cli/package.json
--- a/packages/cli/package.json
+++ b/packages/cli/package.json
@@ -21,3 +21,4 @@
         "semver": "^7.5.2",
-        "yargs": "^17.6.2"
+        "yargs": "^17.6.2",
+        "lodash": "^4.17.21"
     },
EOF
@@ -21,3 +21,4 @@
"semver": "^7.5.2",
"yargs": "^17.6.2"
"yargs": "^17.6.2",
"lodash": "^4.17.21"
},
This fix introduces these dependencies
Package Version Security advisories
lodash (npm) 4.17.21 None
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@@ -40,7 +40,7 @@
export class Upload {
private readonly states: StatesRedisClient;
private readonly objects: ObjectsRedisClient;
private readonly regApp = new RegExp('/' + tools.appName.replace(/\./g, '\\.') + '\\.', 'i');
private readonly regApp = new RegExp(`/${tools.appName.replace(/\./g, '\\.')}\\.`, 'i');

Check failure

Code scanning / CodeQL

Incomplete string escaping or encoding High

This does not escape backslash characters in the input.

Copilot Autofix AI 6 months ago

To fix the problem, we need to ensure that all backslashes in the tools.appName string are properly escaped before replacing periods with escaped periods. This can be achieved by first replacing all backslashes with double backslashes and then replacing periods with escaped periods. This ensures that the resulting string is safe to use in a regular expression.

  • Modify the tools.appName.replace call to first escape backslashes.
  • Ensure that the regular expression is constructed correctly with the properly escaped string.
Suggested changeset 1
packages/cli/src/lib/setup/setupUpload.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/cli/src/lib/setup/setupUpload.ts b/packages/cli/src/lib/setup/setupUpload.ts
--- a/packages/cli/src/lib/setup/setupUpload.ts
+++ b/packages/cli/src/lib/setup/setupUpload.ts
@@ -42,3 +42,3 @@
     private readonly objects: ObjectsRedisClient;
-    private readonly regApp = new RegExp(`/${tools.appName.replace(/\./g, '\\.')}\\.`, 'i');
+    private readonly regApp = new RegExp(`/${tools.appName.replace(/\\/g, '\\\\').replace(/\./g, '\\.')}\\.`, 'i');
     private callbackId = 1;
EOF
@@ -42,3 +42,3 @@
private readonly objects: ObjectsRedisClient;
private readonly regApp = new RegExp(`/${tools.appName.replace(/\./g, '\\.')}\\.`, 'i');
private readonly regApp = new RegExp(`/${tools.appName.replace(/\\/g, '\\\\').replace(/\./g, '\\.')}\\.`, 'i');
private callbackId = 1;
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
function _emit_(id, obj) {
result.rows.push({ id: id, value: obj });
}

const f = eval('(' + func.map.replace(/emit/g, '_emit_') + ')');
const f = eval(`(${func.map.replace(/emit/g, '_emit_')})`);

Check failure

Code scanning / CodeQL

Code injection Critical

This code execution depends on a
user-provided value
.

Copilot Autofix AI 6 months ago

To fix the code injection vulnerability, we should avoid using eval to execute user-provided data. Instead, we can use the Function constructor to create a new function in a safer manner. This approach allows us to control the scope and avoid executing arbitrary code.

  • Replace the eval function with the Function constructor.
  • Ensure that the new function is created in a controlled environment to prevent code injection.
  • Update the _applyView method to use the Function constructor.
Suggested changeset 1
packages/db-objects-file/src/lib/objects/objectsInMemFileDB.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/db-objects-file/src/lib/objects/objectsInMemFileDB.js b/packages/db-objects-file/src/lib/objects/objectsInMemFileDB.js
--- a/packages/db-objects-file/src/lib/objects/objectsInMemFileDB.js
+++ b/packages/db-objects-file/src/lib/objects/objectsInMemFileDB.js
@@ -952,3 +952,3 @@
 
-        const f = eval(`(${func.map.replace(/emit/g, '_emit_')})`);
+        const f = new Function('_emit_', `return (${func.map.replace(/emit/g, '_emit_')});`)(_emit_);
 
EOF
@@ -952,3 +952,3 @@

const f = eval(`(${func.map.replace(/emit/g, '_emit_')})`);
const f = new Function('_emit_', `return (${func.map.replace(/emit/g, '_emit_')});`)(_emit_);

Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@foxriver76 foxriver76 marked this pull request as ready for review September 23, 2024 07:24
@foxriver76 foxriver76 merged commit 4e8189a into master Sep 23, 2024
11 of 12 checks passed
@foxriver76 foxriver76 deleted the eslint-config branch September 23, 2024 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants