infra: Cleaning (#19)

.github/PULL_REQUEST_TEMPLATE.md

@@ -24,3 +24,4 @@ You can check the items by adding an `x` between the brackets, like this: `[x]`
 - [ ] I have formatted using `npx turbo format`
 - [ ] I have checked the types using `npx turbo type-check`
 - [ ] I have tested the build using `npx turbo build`
+- [ ] I have covered the code with tests if necessary

.github/workflows/deploy-storybook.yml

@@ -4,7 +4,7 @@ name: Deploy to GitHub Pages
 on:
   # Runs on pushes targeting the default branch
   push:
-    branches: ["main"]
+    branches: 'main'
 
   # Allows you to run this workflow manually from the Actions tab
   workflow_dispatch:
@@ -18,7 +18,7 @@ permissions:
 # Allow only one concurrent deployment, skipping runs queued between the run in-progress and latest queued.
 # However, do NOT cancel in-progress runs as we want to allow these production deployments to complete.
 concurrency:
-  group: "pages"
+  group: 'pages'
   cancel-in-progress: false
 
 jobs:
@@ -31,14 +31,14 @@ jobs:
     steps:
       - name: Checkout
         uses: actions/checkout@v3
-      - run: npm i
+      - run: npm ci
       - run: npm run storybook:build
       - name: Setup Pages
         uses: actions/configure-pages@v3
       - name: Upload artifact
         uses: actions/upload-pages-artifact@v2
         with:
-          path: "./storybook-static"
+          path: './storybook-static'
       - name: Deploy to GitHub Pages
         id: deployment
         uses: actions/deploy-pages@v2

.github/workflows/scorecard.yml

@@ -8,7 +8,7 @@ on:
   schedule:
     - cron: '45 10 * * 1'
   push:
-    branches: ['main']
+    branches: 'main'
 
 # Declare default permissions as read only.
 permissions: read-all

backend/Setting.ts

@@ -1,57 +1,26 @@
-'use strict';
-/**
- * The Settings module reads the settings out of settings.json and provides
- * this information to the other modules
- *
- * TODO muxator 2020-04-14:
- *
- * 1) get rid of the reloadSettings() call at module loading;
- * 2) provide a factory method that configures the settings module at runtime,
- *    reading the file name either from command line parameters, from a function
- *    argument, or falling back to a default.
- */
-
-/*
- * 2011 Peter 'Pita' Martischka (Primary Technology Ltd)
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS-IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import fs from 'fs';
-import os from 'os';
-import path from 'path';
+import fs from 'node:fs';
+import os from 'node:os';
+import path from 'node:path';
 import pinoLogger from 'pino';
 import { SettingsObj } from '@/types/SettingsObject';
 import { findEtherpadRoot, makeAbsolute } from '@/utils/backend/AbsolutePaths';
 import { argvP } from '@/utils/backend/CLI';
-const suppressDisableMsg =
-  ' -- To suppress these warning messages change ' +
-  'suppressErrorsInPadText to true in your settings.json\n';
 import minify from '@/utils/backend/minify';
 
+const suppressDisableMsg = ` -- To suppress these warning messages change suppressErrorsInPadText to true in your settings.json`;
+
 // Exported values that settings.json and credentials.json cannot override.
 const nonSettings = ['credentialsFilename', 'settingsFilename'];
 
-let logger: any|undefined = undefined;
+let logger: any | undefined;
 
 const defaultLogLevel = 'INFO';
 
 const initLogging = (level: string) => {
-
   logger = pinoLogger({
     level: level.toLowerCase(),
     transport: {
-      target: 'pino-pretty'
+      target: 'pino-pretty',
     },
   });
   // Overwrites for console output methods
@@ -136,14 +105,14 @@ export const settings: SettingsObj = {
   /**
    * The default Text of a new pad
    */
-  defaultPadText: [
-    'Welcome to Etherpad!',
-    '',
-    'This pad text is synchronized as you type, so that everyone viewing this page sees the same ' +
-      'text. This allows you to collaborate seamlessly on documents!',
-    '',
-    'Etherpad on Github: https://github.com/ether/etherpad-lite',
-  ].join('\n'),
+  defaultPadText: `
+    Welcome to Etherpad!
+
+    This pad text is synchronized as you type, so that everyone viewing this page sees the same
+    text. This allows you to collaborate seamlessly on documents!
+
+    Etherpad on Github: https://github.com/ether/etherpad-lite
+  `,
   padOptions: {
     noColors: false,
     showControls: true,
@@ -471,7 +440,7 @@ export const settings: SettingsObj = {
       if (settings[i] !== undefined || i.indexOf('ep_') === 0) {
         if (
           // @ts-ignore
-        typeof settingsObj[i] == 'object' &&
+          typeof settingsObj[i] == 'object' &&
           // @ts-ignore
           !Array.isArray(settingsObj[i])
         ) {
@@ -508,7 +477,8 @@ export const settings: SettingsObj = {
     // cooked from https://stackoverflow.com/questions/175739/built-in-way-in-javascript-to-check-if-a-string-is-a-valid-number
     const isNumeric =
       !isNaN(Number(stringValue)) &&
-      !isNaN(parseFloat(stringValue)) && isFinite(Number(stringValue));
+      !isNaN(parseFloat(stringValue)) &&
+      isFinite(Number(stringValue));
 
     if (isNumeric) {
       // detected numeric string. Coerce to a number
@@ -611,10 +581,10 @@ export const settings: SettingsObj = {
 
       if (envVarValue === undefined && defaultValue === undefined) {
         logger.warn(
-          `Environment variable "${envVarName}" does not contain any value for ` +
-            `configuration key "${key}", and no default was given. Using null. ` +
-            'THIS BEHAVIOR MAY CHANGE IN A FUTURE VERSION OF ETHERPAD; you should ' +
-            'explicitly use "null" as the default if you want to continue to use null.'
+          `Environment variable "${envVarName}" does not contain any value for
+            configuration key "${key}", and no default was given. Using null.
+            THIS BEHAVIOR MAY CHANGE IN A FUTURE VERSION OF ETHERPAD; you should
+            explicitly use "null" as the default if you want to continue to use null.`
         );
 
         /*
@@ -626,8 +596,7 @@ export const settings: SettingsObj = {
 
       if (envVarValue === undefined && defaultValue !== undefined) {
         logger.debug(
-          `Environment variable "${envVarName}" not found for ` +
-            `configuration key "${key}". Falling back to default value.`
+          `Environment variable "${envVarName}" not found for configuration key "${key}". Falling back to default value.`
         );
 
         return settings.coerceValue(defaultValue);
@@ -694,8 +663,7 @@ export const settings: SettingsObj = {
       return settings.lookupEnvironmentVariables(settingsParsed);
     } catch (e: any) {
       logger.error(
-        `There was an error processing your ${settingsType} ` +
-          `file from ${settingsFilename}: ${e.message}`
+        `There was an error processing your ${settingsType} file from ${settingsFilename}: ${e.message}`
       );
 
       process.exit(1);
@@ -721,9 +689,11 @@ export const settings: SettingsObj = {
         if (doesNotExist) {
           const abiwordError =
             'Abiword does not exist at this path, check your settings file.';
+
           if (!settings.suppressErrorsInPadText) {
             settings.defaultPadText += `\nError: ${abiwordError}${suppressDisableMsg}`;
           }
+
           logger.error(`${abiwordError} File location: ${settings.abiword}`);
           settings.abiword = null;
         }
@@ -739,6 +709,7 @@ export const settings: SettingsObj = {
           if (!settings.suppressErrorsInPadText) {
             settings.defaultPadText += `\nError: ${sofficeError}${suppressDisableMsg}`;
           }
+
           logger.error(`${sofficeError} File location: ${settings.soffice}`);
           settings.soffice = null;
         }
@@ -748,6 +719,7 @@ export const settings: SettingsObj = {
     if (settings.dbType === 'dirty') {
       const dirtyWarning =
         'DirtyDB is used. This is not recommended for production.';
+
       if (!settings.suppressErrorsInPadText) {
         settings.defaultPadText += `\nWarning: ${dirtyWarning}${suppressDisableMsg}`;
       }
@@ -761,8 +733,7 @@ export const settings: SettingsObj = {
     if (settings.ip === '') {
       // using Unix socket for connectivity
       logger.warn(
-        'The settings file contains an empty string ("") for the "ip" parameter. The ' +
-          '"port" parameter will be interpreted as the path to a Unix socket to bind at.'
+        `The settings file contains an empty string ("") for the "ip" parameter. The "port" parameter will be interpreted as the path to a Unix socket to bind at.`
       );
     }
     return settings;
@@ -771,8 +742,7 @@ export const settings: SettingsObj = {
 
 /* Root path of the installation */
 logger.info(
-  'All relative paths will be interpreted relative to the identified ' +
-    `Etherpad base dir: ${root}`
+  `All relative paths will be interpreted relative to the identified Etherpad base dir: ${root}`
 );
 
 export default settings;

backend/socketio.ts

@@ -1,14 +1,14 @@
-import events from 'events';
+import events from 'node:events';
 import { Http2Server } from 'node:http2';
 import { Server, Socket } from 'socket.io';
 import { DefaultEventsMap } from '@socket.io/component-emitter';
 
+let io: Server<DefaultEventsMap, DefaultEventsMap> | undefined;
 const sockets = new Set();
-export const socketsEvents = new events.EventEmitter();
-
 const sessionInfos = new Map();
 
-let io: Server<DefaultEventsMap, DefaultEventsMap> | undefined;
+export const socketsEvents = new events.EventEmitter();
+
 export const initSocketIO = (server: Http2Server) => {
   io = new Server(server, { addTrailingSlash: false });
 

package.json

@@ -38,7 +38,6 @@
     "@storybook/addon-themes": "~7.6.17",
     "@storybook/nextjs": "~7.6.17",
     "@storybook/react": "~7.6.10",
-    "@types/find-root": "~1.1.4",
     "@types/node": "~20.11.20",
     "@types/react": "~18.2.58",
     "@types/react-dom": "~18.2.19",

server.ts

@@ -3,6 +3,7 @@ import { parse } from 'node:url';
 import next from 'next';
 import { initSocketIO } from '@/backend/socketio';
 import settings from '@/backend/Setting';
+
 const dev = process.env.NODE_ENV !== 'production';
 const hostname = 'localhost';
 // when using middleware `hostname` and `port` must be provided below

tests/settings/settings.spec.test.ts

@@ -1,8 +1,8 @@
 import assert from 'node:assert/strict';
-import settingsMod from '@/backend/Setting';
-import path from 'path';
-import process from 'process';
+import path from 'node:path';
+import process from 'node:process';
 import { describe, it, beforeEach } from 'vitest';
+import settingsMod from '@/backend/Setting';
 
 describe(__filename, function () {
   describe('parseSettings', function () {

tests/utils/backend/AbsolutePaths.test.ts

@@ -0,0 +1,42 @@
+import path from 'node:path';
+import assert from 'node:assert/strict';
+import { describe, it } from 'vitest';
+import {
+  findEtherpadRoot,
+  makeAbsolute,
+  isSubdir,
+} from '@/utils/backend/AbsolutePaths';
+
+describe('AbsolutePaths', () => {
+  it('findEtherpadRoot - returns current working directory if etherpadRoot is not set', () => {
+    const originalCwd = process.cwd();
+    const result = findEtherpadRoot();
+    assert.equal(result, originalCwd);
+  });
+
+  it('makeAbsolute - returns absolute path if already absolute', () => {
+    const absolutePath = '/some/absolute/path';
+    const result = makeAbsolute(absolutePath);
+    assert.equal(result, absolutePath);
+  });
+
+  it('isSubdir - returns true for a subdirectory', () => {
+    const parent = '/parent/directory';
+    const subdirectory = '/parent/directory/subdir';
+    const result = isSubdir(parent, subdirectory);
+    assert.strictEqual(result, true);
+  });
+
+  it('isSubdir - returns false for a directory outside parent', () => {
+    const parent = '/parent/directory';
+    const outsideDirectory = '/outside/directory';
+    const result = isSubdir(parent, outsideDirectory);
+    assert.strictEqual(result, false);
+  });
+
+  it('isSubdir - returns false for parent equal to arbitrary directory', () => {
+    const directory = '/some/directory';
+    const result = isSubdir(directory, directory);
+    assert.strictEqual(result, false);
+  });
+});