ci(js): lint the backend JavaScript, which nothing did

A file whose methods had landed outside their class passed the whole CI
matrix — 71 green checks — and the rating buttons it shipped would not
have worked. PHPStan does not read JavaScript, the functional suite
renders the template without executing the module, and no Playwright spec
covers that view. A person reading the diff found it.

node --check was the cheaper candidate and lost on evidence, not on
taste: it parses a .js file as a CommonJS script and ACCEPTED the broken
file, while ESLint parses it as an ES module — which is how the browser
loads it — and reports the parse error on the right line. That also
settles a contradiction left open in #842's review: the file was a syntax
error all along; node --check simply read it in the wrong mode.

My stated reason for not deciding this earlier was wrong, and checking it
took one command: "eslint would bring a JS toolchain into a PHP
extension". package.json, package-lock.json and three devDependencies
were already there, and CI already runs npm install for the e2e job.

No sibling extension lints JavaScript — rte_ckeditor_image has a
package.json for commitlint only, nr-vault one for Playwright,
nr-repurpose none. There was no house standard to follow, which is a
reason to set one rather than to wait.

The rule set is small on purpose. no-undef is the error, because that is
the rule that catches the defect above. no-unsanitized is loaded but set
to warn: as an error it reports 12 assignments and NONE is a hole — the
code escapes through escapeHtml(), visibly and with a comment saying so,
and the rule cannot recognise a helper of ours as a sanitizer. Dropping
the plugin was not an option either: two files already carry
eslint-disable-line no-unsanitized/property, suppressions written for a
rule that never ran, which become "rule not found" errors the moment
linting is switched on. Those twelve are triaged in #854 with the
evidence, because a security question belongs where it can be argued.

The clean tree lints at 0 errors. Reintroducing the #825 shape — a method
after the class's closing brace — fails with a parse error and exit 1.

Thin caller of the shared script-check reusable, so no job is defined
here; pre-command installs the devDependencies the runner does not ship.

Closes #825

Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de>

Author: CybotTM

.github/workflows/js-lint.yml

@@ -0,0 +1,34 @@
+name: JavaScript Lint
+
+# Lints Resources/Public/JavaScript. Nothing did before #825: a file whose
+# methods had landed outside their class passed the whole CI matrix — 71 green
+# checks — because PHPStan does not read JavaScript, the functional suite
+# renders the template without executing the module, and no Playwright spec
+# covers that view. The defect was found by a person reading the diff.
+#
+# `node --check` was the cheaper candidate and lost on evidence: it parses a
+# `.js` file as a CommonJS script and accepted the broken file, while ESLint
+# parses it as an ES module — which is how the browser loads it — and reports
+# the parse error on the right line.
+#
+# Thin caller of the shared script-check reusable, so the checkout pin and
+# harden-runner stay maintained centrally. `pre-command` installs the npm
+# devDependencies the runner does not ship.
+
+on:
+  push:
+    branches: [main]
+  pull_request:
+  merge_group:
+  workflow_dispatch:
+
+permissions: {}
+
+jobs:
+  js-lint:
+    uses: netresearch/.github/.github/workflows/script-check.yml@main
+    permissions:
+      contents: read
+    with:
+      pre-command: npm ci --no-audit --no-fund
+      check-cmd: npx eslint

eslint.config.mjs

@@ -0,0 +1,62 @@
+// Lints the backend JavaScript. Nothing did before: a file whose methods had
+// landed outside their class passed the whole CI matrix — 71 green checks —
+// because PHPStan does not read JavaScript, the functional suite renders the
+// template without executing the module, and no Playwright spec covers that
+// view (#825).
+//
+// `node --check` was considered as a cheaper floor and rejected on evidence:
+// it parses a `.js` file as a CommonJS script and accepted the broken file,
+// while ESLint parses it as an ES module — which is how the browser loads it —
+// and reports the parse error on the right line.
+//
+// The rule set is deliberately small. This is the first JavaScript gate in any
+// of our TYPO3 extensions, and a large one would arrive as a backlog of
+// pre-existing violations that nobody asked for.
+import globals from 'globals';
+import noUnsanitized from 'eslint-plugin-no-unsanitized';
+
+export default [
+    {
+        files: ['Resources/Public/JavaScript/**/*.js'],
+        // Vendored libraries are shipped as-is; linting them would report
+        // somebody else's code in our gate.
+        ignores: ['Resources/Public/JavaScript/Vendor/**'],
+        languageOptions: {
+            ecmaVersion: 2022,
+            sourceType: 'module',
+            globals: {
+                ...globals.browser,
+                // Provided by the TYPO3 backend at runtime, not importable.
+                TYPO3: 'readonly',
+                bootstrap: 'readonly',
+            },
+        },
+        plugins: { 'no-unsanitized': noUnsanitized },
+        rules: {
+            // The rule that would have caught #825.
+            'no-undef': 'error',
+            'no-unused-vars': ['warn', { argsIgnorePattern: '^_' }],
+            // Warning, not error, and the reason is measured rather than
+            // cautious. Switched on as an error it reports 12 assignments —
+            // and none of them is a hole: the code escapes first, visibly, at
+            // `SetupWizard.js:471-474` ("SECURITY: Escape all external data")
+            // through `escapeHtml()`. The rule cannot recognise that helper as
+            // a sanitizer, which is a known shape for it.
+            //
+            // The plugin is still loaded, because two files carry
+            // `eslint-disable-line no-unsanitized/property` — suppressions
+            // written for a rule that never ran. Without the plugin those
+            // comments become "rule not found" errors the moment linting is
+            // switched on.
+            //
+            // Making them errors would land a backlog of twelve judgements
+            // nobody asked for on a change whose job is #825. Leaving the rule
+            // out entirely would drop a real check silently. The findings are
+            // triaged in their own issue, with the escapeHtml evidence, so the
+            // security question lives somewhere it can be argued rather than
+            // in a config comment.
+            'no-unsanitized/property': 'warn',
+            'no-unsanitized/method': 'warn',
+        },
+    },
+];