diff --git a/.github/eslint-plugin/index.js b/.github/eslint-plugin/index.js new file mode 100644 index 0000000000..462a0578ab --- /dev/null +++ b/.github/eslint-plugin/index.js @@ -0,0 +1,86 @@ +#!/usr/bin/env node + +// eslint plugin rule utility +// ============================ +// node index.js generate-menu +// generates the _menu.md file to make sure all rules are included. +// node index.js generate-js-stub +// generates a stub markdown file for a new JS rule with name . + +import * as fs from 'node:fs' +import * as path from 'node:path' + +const RULES_BASE_PATH = path.join('tools', 'cds-lint', 'rules'); +const EXAMPLES_BASE_PATH = path.join('tools', 'cds-lint', 'examples'); +const MENU_FILE_NAME = '_menu.md'; + +/** + * Get a list of all rule description files. + * @returns {string[]} An array of rule description file names. + */ +const getRuleDescriptionFiles = () => + fs.readdirSync(RULES_BASE_PATH) + .filter(file => file.endsWith('.md')) + .filter(file => !['index.md', MENU_FILE_NAME].includes(file)) + .sort() + +/** + * Generates the menu markdown file + * by completely overriding its current contents. + * The menu contains links to all rule description files + * in alphabetical order. + */ +function generateMenuMarkdown () { + const rules = getRuleDescriptionFiles(); + const menu = rules.map(rule => { + const clean = rule.replace('.md', ''); + return `# [${clean}](${clean})` + }).join('\n'); + const menuFilePath = path.join(RULES_BASE_PATH, '_menu.md') + fs.writeFileSync(menuFilePath, menu); + console.info(`generated menu to ${menuFilePath}`) +} + +/** + * Generates a stub markdown file for a new JS rule. + * The passed ruleName will be placed in the stub template + * where $RULE_NAME is defined. + * @param {string} ruleName - The name of the rule. + */ +function generateJsRuleStub (ruleName) { + if (!ruleName) { + console.error('Please provide a rule name, e.g. "no-shared-handler-variables" as second argument'); + process.exit(1); + } + const stubFilePath = path.join(RULES_BASE_PATH, ruleName + '.md'); + if (fs.existsSync(stubFilePath)) { + console.error(`file ${stubFilePath} already exists, will not overwrite`); + process.exit(2); + } + const stub = fs.readFileSync(path.join(import.meta.dirname, 'js-rule-stub.md'), 'utf-8').replaceAll('$RULE_NAME', ruleName); + fs.writeFileSync(stubFilePath, stub); + console.info(`generated stub to ${stubFilePath}`); + const correctPath = path.join(EXAMPLES_BASE_PATH, ruleName, 'correct', 'srv'); + fs.mkdirSync(correctPath, { recursive: true }); + const incorrectPath = path.join(EXAMPLES_BASE_PATH, ruleName, 'incorrect', 'srv'); + fs.mkdirSync(incorrectPath, { recursive: true }); + console.info(`generated example directories in ${path.join(EXAMPLES_BASE_PATH, ruleName)}`); + fs.writeFileSync(path.join(correctPath, 'admin-service.js'), '// correct example\n'); + fs.writeFileSync(path.join(incorrectPath, 'admin-service.js'), '// incorrect example\n'); +} + +function main (argv) { + switch (argv[0]) { + case 'generate-menu': + generateMenuMarkdown(); + break; + case 'generate-js-stub': + generateJsRuleStub(argv[1]); + generateMenuMarkdown(); + break; + default: + console.log(`Unknown command: ${argv[0]}. Use one of: generate-menu, generate-stub`); + } +} + +main(process.argv.slice(2)); \ No newline at end of file diff --git a/.github/eslint-plugin/js-rule-stub.md b/.github/eslint-plugin/js-rule-stub.md new file mode 100644 index 0000000000..6cdbe627bc --- /dev/null +++ b/.github/eslint-plugin/js-rule-stub.md @@ -0,0 +1,44 @@ +--- +status: released +--- + + + +# $RULE_NAME + +## Rule Details + +DETAILS + +#### Version +This rule was introduced in `@sap/eslint-plugin-cds x.y.z`. + +## Examples + +### ✅   Correct example + +DESCRIPTION OF CORRECT EXAMPLE + +::: code-group +<<< ../examples/$RULE_NAME/correct/srv/admin-service.js#snippet{js:line-numbers} [srv/admin-service.js] +::: + + +### ❌   Incorrect example + +DESCRIPTION OF INCORRECT EXAMPLE + +::: code-group +<<< ../examples/$RULE_NAME/incorrect/srv/admin-service.js#snippet{js:line-numbers} [srv/admin-service.js] +::: + diff --git a/tools/cds-lint/examples/case-sensitive-well-known-events/correct/srv/admin-service.js b/tools/cds-lint/examples/case-sensitive-well-known-events/correct/srv/admin-service.js new file mode 100644 index 0000000000..0d6060edfc --- /dev/null +++ b/tools/cds-lint/examples/case-sensitive-well-known-events/correct/srv/admin-service.js @@ -0,0 +1,5 @@ +const cds = require('@sap/cds') + +module.exports = class AdminService extends cds.ApplicationService { async init() { + this.on('READ', 'Books', () => {}) // [!code highlight] +}} \ No newline at end of file diff --git a/tools/cds-lint/examples/case-sensitive-well-known-events/incorrect/srv/admin-service.js b/tools/cds-lint/examples/case-sensitive-well-known-events/incorrect/srv/admin-service.js new file mode 100644 index 0000000000..7871899832 --- /dev/null +++ b/tools/cds-lint/examples/case-sensitive-well-known-events/incorrect/srv/admin-service.js @@ -0,0 +1,5 @@ +const cds = require('@sap/cds') + +module.exports = class AdminService extends cds.ApplicationService { async init() { + this.on('Read', 'Books', () => {}) // [!code error] +}} \ No newline at end of file diff --git a/tools/cds-lint/examples/no-cross-service-import/correct/srv/AdminService.js b/tools/cds-lint/examples/no-cross-service-import/correct/srv/AdminService.js new file mode 100644 index 0000000000..85966e6628 --- /dev/null +++ b/tools/cds-lint/examples/no-cross-service-import/correct/srv/AdminService.js @@ -0,0 +1,6 @@ +const cds = require('@sap/cds') +const { Books } = require('#cds-models/sap/capire/bookshop/AdminService') // [!code highlight] + +module.exports = class AdminService extends cds.ApplicationService { + // … +} \ No newline at end of file diff --git a/tools/cds-lint/examples/no-cross-service-import/incorrect/srv/AdminService.js b/tools/cds-lint/examples/no-cross-service-import/incorrect/srv/AdminService.js new file mode 100644 index 0000000000..f8cb922c75 --- /dev/null +++ b/tools/cds-lint/examples/no-cross-service-import/incorrect/srv/AdminService.js @@ -0,0 +1,6 @@ +const cds = require('@sap/cds') +const { Books } = require('#cds-models/sap/capire/bookshop/CatalogService') // [!code error] + +module.exports = class AdminService extends cds.ApplicationService { + // … +} \ No newline at end of file diff --git a/tools/cds-lint/examples/no-deep-sap-cds-import/correct/srv/admin-service.js b/tools/cds-lint/examples/no-deep-sap-cds-import/correct/srv/admin-service.js new file mode 100644 index 0000000000..c62134c0d6 --- /dev/null +++ b/tools/cds-lint/examples/no-deep-sap-cds-import/correct/srv/admin-service.js @@ -0,0 +1,5 @@ +const cds = require('@sap/cds') // [!code highlight] + +module.exports = class AdminService extends cds.ApplicationService { + // … +} \ No newline at end of file diff --git a/tools/cds-lint/examples/no-deep-sap-cds-import/incorrect/srv/admin-service.js b/tools/cds-lint/examples/no-deep-sap-cds-import/incorrect/srv/admin-service.js new file mode 100644 index 0000000000..13658befaf --- /dev/null +++ b/tools/cds-lint/examples/no-deep-sap-cds-import/incorrect/srv/admin-service.js @@ -0,0 +1,5 @@ +const cdsService = require('@sap/cds/service') // [!code error] + +module.exports = class AdminService extends cdsService.ApplicationService { + // … +} \ No newline at end of file diff --git a/tools/cds-lint/examples/no-shared-handler-variable/correct/srv/admin-service.js b/tools/cds-lint/examples/no-shared-handler-variable/correct/srv/admin-service.js new file mode 100644 index 0000000000..9fed76fc34 --- /dev/null +++ b/tools/cds-lint/examples/no-shared-handler-variable/correct/srv/admin-service.js @@ -0,0 +1,21 @@ +const cds = require('@sap/cds') + +module.exports = class AdminService extends cds.ApplicationService { async init() { + this.after('READ', 'Books', async () => { + // local variable only, no state shared between handlers + const books = await cds.run(SELECT.from('Books')) // [!code highlight] + return books + }) + + this.on('CREATE', 'Books', newBookHandler) + await super.init() + } +} + +/** @type {import('@sap/cds').CRUDEventHandler.On} */ +async function newBookHandler (req) { + const { name } = req.data + // local variable only, no state shared between handlers + const newBook = await cds.run(INSERT.into('Books').entries({ name })) // [!code highlight] + return newBook +} \ No newline at end of file diff --git a/tools/cds-lint/examples/no-shared-handler-variable/incorrect/srv/admin-service.js b/tools/cds-lint/examples/no-shared-handler-variable/incorrect/srv/admin-service.js new file mode 100644 index 0000000000..827ab536ff --- /dev/null +++ b/tools/cds-lint/examples/no-shared-handler-variable/incorrect/srv/admin-service.js @@ -0,0 +1,24 @@ +const cds = require('@sap/cds') + +let lastCreatedBook +let lastReadBooks + +module.exports = class AdminService extends cds.ApplicationService { async init() { + this.after('READ', 'Books', async () => { + // variable from surrounding scope, state is shared between handler calls + lastReadBooks = await cds.run(SELECT.from('Books')) // [!code error] + return lastReadBooks + }) + + this.on('CREATE', 'Books', newBookHandler) + await super.init() + } +} + +/** @type {import('@sap/cds').CRUDEventHandler.On} */ +async function newBookHandler (req) { + const { name } = req.data + // variable from surrounding scope, state is shared between handler calls + lastCreatedBook = await cds.run(INSERT.into('Books').entries({ name })) // [!code error] + return lastCreatedBook +} \ No newline at end of file diff --git a/tools/cds-lint/examples/use-cql-select-template-strings/correct/srv/admin-service.js b/tools/cds-lint/examples/use-cql-select-template-strings/correct/srv/admin-service.js new file mode 100644 index 0000000000..3d46adad9b --- /dev/null +++ b/tools/cds-lint/examples/use-cql-select-template-strings/correct/srv/admin-service.js @@ -0,0 +1,10 @@ +const cds = require('@sap/cds') +module.exports = class AdminService extends cds.ApplicationService { init() { + const { Authors } = cds.entities('AdminService') + + this.before (['CREATE', 'UPDATE'], Authors, async (req) => { + await SELECT`ID`.from `Authors`.where `name = ${req.data.name}` // [!code highlight] + }) + + return super.init() +}} diff --git a/tools/cds-lint/examples/use-cql-select-template-strings/incorrect/srv/admin-service.js b/tools/cds-lint/examples/use-cql-select-template-strings/incorrect/srv/admin-service.js new file mode 100644 index 0000000000..62db419e9f --- /dev/null +++ b/tools/cds-lint/examples/use-cql-select-template-strings/incorrect/srv/admin-service.js @@ -0,0 +1,10 @@ +const cds = require('@sap/cds') +module.exports = class AdminService extends cds.ApplicationService { init() { + const { Authors } = cds.entities('AdminService') + + this.before (['CREATE', 'UPDATE'], Authors, async (req) => { + await SELECT`ID`.from `Authors`.where (`name = ${req.data.name}`) // [!code error] + }) + + return super.init() +}} diff --git a/tools/cds-lint/rules/_menu.md b/tools/cds-lint/rules/_menu.md index 5dab014d3d..e38c79a4aa 100644 --- a/tools/cds-lint/rules/_menu.md +++ b/tools/cds-lint/rules/_menu.md @@ -6,14 +6,19 @@ # [auth-valid-restrict-keys](auth-valid-restrict-keys) # [auth-valid-restrict-to](auth-valid-restrict-to) # [auth-valid-restrict-where](auth-valid-restrict-where) +# [case-sensitive-well-known-events](case-sensitive-well-known-events) # [extension-restrictions](extension-restrictions) # [latest-cds-version](latest-cds-version) +# [no-cross-service-import](no-cross-service-import) # [no-db-keywords](no-db-keywords) +# [no-deep-sap-cds-import](no-deep-sap-cds-import) # [no-dollar-prefixed-names](no-dollar-prefixed-names) # [no-java-keywords](no-java-keywords) # [no-join-on-draft](no-join-on-draft) +# [no-shared-handler-variable](no-shared-handler-variable) # [sql-cast-suggestion](sql-cast-suggestion) # [sql-null-comparison](sql-null-comparison) # [start-elements-lowercase](start-elements-lowercase) # [start-entities-uppercase](start-entities-uppercase) -# [valid-csv-header](valid-csv-header) +# [use-cql-select-template-strings](use-cql-select-template-strings) +# [valid-csv-header](valid-csv-header) \ No newline at end of file diff --git a/tools/cds-lint/rules/case-sensitive-well-known-events.md b/tools/cds-lint/rules/case-sensitive-well-known-events.md new file mode 100644 index 0000000000..52b1277295 --- /dev/null +++ b/tools/cds-lint/rules/case-sensitive-well-known-events.md @@ -0,0 +1,43 @@ +--- +status: released +--- + + + +# case-sensitive-well-known-events + +## Rule Details + +This rule identifies registrations to events that are likely well-known event names that must be written in all caps. + +#### Version +This rule was introduced in `@sap/eslint-plugin-cds 4.0.2`. + +## Examples + +### ✅   Correct example + +The following example shows the correctly capitalized event name `READ`: + +::: code-group +<<< ../examples/case-sensitive-well-known-events/correct/srv/admin-service.js#snippet{js:line-numbers} [srv/admin-service.js] +::: + + +### ❌   Incorrect example + +This example shows a registration to an event `Read`, which should likely be `READ`. This can lead to unexpected behavior because event names in CAP are case sensitive: +::: code-group +<<< ../examples/case-sensitive-well-known-events/incorrect/srv/admin-service.js#snippet{js:line-numbers} [srv/admin-service.js] +::: + diff --git a/tools/cds-lint/rules/index.md b/tools/cds-lint/rules/index.md index 502cafa78f..b89e50377d 100644 --- a/tools/cds-lint/rules/index.md +++ b/tools/cds-lint/rules/index.md @@ -14,7 +14,7 @@ status: released Below you can find all rules of the `@sap/eslint-plugin-cds` ESLint plugin. -They are grouped by categories [Model Validation](#model-validation) and [Environment](#environment) to help you understand their purpose. +They are grouped by categories [Model Validation](#model-validation), [JavaScript](#javascript), and [Environment](#environment) to help you understand their purpose. Your standard CDS project configuration turns on a subset of these rules by default, namely the *recommended* ( ✅ ) rules to ensure basic standards are met. diff --git a/tools/cds-lint/rules/no-cross-service-import.md b/tools/cds-lint/rules/no-cross-service-import.md new file mode 100644 index 0000000000..41e874b96e --- /dev/null +++ b/tools/cds-lint/rules/no-cross-service-import.md @@ -0,0 +1,43 @@ +--- +status: released +--- + + + +# no-cross-service-import + +## Rule Details + +This rule prevents importing artifacts generated for one service into another service's implementation when using cds-typer. Clear service boundaries make your codebase easier to maintain and understand. + +#### Version +This rule was introduced in `@sap/eslint-plugin-cds 4.0.2`. + +## Examples + +### ✅   Correct example + +The imported entity belongs to `AdminService` and is used within the implementation of `AdminService` itself. This is the recommended approach: +::: code-group +<<< ../examples/no-cross-service-import/correct/srv/AdminService.js#snippet{js:line-numbers} [srv/AdminService.js] +::: + + +### ❌   Incorrect example + +An entity from `CatalogService` is imported into the implementation of `AdminService`. This cross-service import is discouraged because it can lead to confusion and maintenance issues: + +::: code-group +<<< ../examples/no-cross-service-import/incorrect/srv/AdminService.js#snippet{js:line-numbers} [srv/AdminService.js] +::: + diff --git a/tools/cds-lint/rules/no-deep-sap-cds-import.md b/tools/cds-lint/rules/no-deep-sap-cds-import.md new file mode 100644 index 0000000000..c1e282031f --- /dev/null +++ b/tools/cds-lint/rules/no-deep-sap-cds-import.md @@ -0,0 +1,45 @@ +--- +status: released +--- + + + +# no-deep-sap-cds-import + +## Rule Details + +Import only from the top-level `@sap/cds` package. Accessing internal modules or sub-paths is unsafe because these are not part of the official public API and may change or be removed without notice. +A few exceptions to this rule will not be reported as errors. + +#### Version +This rule was introduced in `@sap/eslint-plugin-cds 4.0.2`. + +## Examples + +### ✅   Correct example + +The following example imports `@sap/cds`: + +::: code-group +<<< ../examples/no-deep-sap-cds-import/correct/srv/admin-service.js#snippet{js:line-numbers} [srv/admin-service.js] +::: + + +### ❌   Incorrect example + +This example incorrectly performs a deep import of a file within `@sap/cds`: + +::: code-group +<<< ../examples/no-deep-sap-cds-import/incorrect/srv/admin-service.js#snippet{js:line-numbers} [srv/admin-service.js] +::: + diff --git a/tools/cds-lint/rules/no-shared-handler-variable.md b/tools/cds-lint/rules/no-shared-handler-variable.md new file mode 100644 index 0000000000..e034969ee4 --- /dev/null +++ b/tools/cds-lint/rules/no-shared-handler-variable.md @@ -0,0 +1,88 @@ +--- +status: released +--- + + + +# no-shared-handler-variable + +## Rule Details + +Discourages sharing state between handlers using variables from parent scopes because this can lead to data leakage between tenants. This rule automatically checks handler registrations inside classes that extend `cds.ApplicationService`. + +To enable this check for functions declared outside such classes, add a type annotation. +Any function annotated with + +- `@type {import('@sap/cds').CRUDEventHandler.Before}`, +- `@type {import('@sap/cds').CRUDEventHandler.On}`, +- or `@type {import('@sap/cds').CRUDEventHandler.After}` + +will also be checked by this rule. + + +## Caveats +The following code styles are not checked by this rule as of today: + +### Inline Import + Extension +The following example imports `@sap/cds` within the `extends` clause of the service implementation. +```js +class AdminService extends require('@sap/cds').ApplicationService { /* … */ } +``` +Instead, import `@sap/cds` separately, as shown in the other examples. + +### Using Methods as Handler +Using a misbehaving class method as handler implementation will also not be detected, even if it is located in service implementation class itself: + +```js +const cds = require('@sap/cds') + +class AdminService extends cds.ApplicationService { + badHandler () { /* bad things going on in here */ } + + init () { + this.on('READ', 'Books', this.badHandler) + } +} +``` +Use a function instead. + +### Other Service-Implementation Styles Than Classes +Only classes extending `cds.ApplicationService` are checked as part of this rule, to avoid triggering too many false positives. So all other implementation styles will not trigger this rule: +```js +cds.services['AdminService'].on('READ', 'Books', () => {}) +``` +(or any of the old implementation patterns for services besides classes, like using `cds.service.impl`.) + + +#### Version +This rule was introduced in `@sap/eslint-plugin-cds 4.0.2`. + +## Examples + +### ✅   Correct example + +In the following example, only locally defined variables are used within handler implementation: + +::: code-group +<<< ../examples/no-shared-handler-variable/correct/srv/admin-service.js#snippet{js:line-numbers} [srv/admin-service.js] +::: + + +### ❌   Incorrect example + +In the following example, the variables `newBook` and `readBooks` are declared in scopes surrounding the handler function, making their value available to subsequent calls of that handler. While this may seem advantageous, it can cause issues in a multitenant scenario, where the handler function can be invoked by multiple tenants. + +::: code-group +<<< ../examples/no-shared-handler-variable/incorrect/srv/admin-service.js#snippet{js:line-numbers} [srv/admin-service.js] +::: + diff --git a/tools/cds-lint/rules/use-cql-select-template-strings.md b/tools/cds-lint/rules/use-cql-select-template-strings.md new file mode 100644 index 0000000000..1d29099aa0 --- /dev/null +++ b/tools/cds-lint/rules/use-cql-select-template-strings.md @@ -0,0 +1,44 @@ +--- +status: released +--- + + + +# use-cql-select-template-strings + +## Rule Details + +Discourages use of SELECT(\`...\`), which allows [SQL injection attacks](../../../node.js/cds-ql#avoiding-sql-injection), in favor of SELECT \`...\`. + +#### Version +This rule was introduced in `@sap/eslint-plugin-cds 4.0.2`. + +## Examples + +### ✅   Correct example + +In the following example, the `where` clause is a proper tagged template literal, so that the `req.data.name` expression can be validated before the SELECT is executed: + +::: code-group +<<< ../examples/use-cql-select-template-strings/correct/srv/admin-service.js#snippet{js:line-numbers} [srv/admin-service.js] +::: + + +### ❌   Incorrect example + +In the following example, the `where` clause is *not* a proper tagged template literal as it's enclosed by parentheses. In consequence, the `req.data.name` expression *cannot* be validated but is added as is to the SELECT statement. This is prone to SQL injection attacks. + +::: code-group +<<< ../examples/use-cql-select-template-strings/incorrect/srv/admin-service.js#snippet{js:line-numbers} [srv/admin-service.js] +::: +