fix: Prevent FileSystemResourceLocker from writing to ./

src/util/locking/FileSystemResourceLocker.ts

@@ -35,10 +35,10 @@ const attemptDefaults: Required<AttemptSettings> = { retryCount: -1, retryDelay:
  * Argument interface of the FileSystemResourceLocker constructor.
  */
 interface FileSystemResourceLockerArgs {
-  /** The rootPath of the filesystem _[default is the current dir `./`]_ */
-  rootFilePath?: string;
+  /** The root filepath of where the server is allowed to write files */
+  rootFilePath: string;
   /**
-   * The path to the directory where locks will be stored (appended to rootFilePath)
+   * The path to the directory where locks will be stored (relative to rootFilePath)
    * _[default is `/.internal/locks`]_
    */
   lockDirectory?: string;
@@ -56,11 +56,12 @@ function isCodedError(err: unknown): err is { code: string } & Error {
  *
  * This **proper-lockfile** library has its own retry mechanism for the operations, since a lock/unlock call will
  * either resolve successfully or reject immediately with the causing error. The retry function of the library
- * however will be ignored and replaced by our own LockUtils' {@link retryFunctionUntil} function.
+ * however will be ignored and replaced by our own LockUtils' {@link retryFunction} function.
  */
 export class FileSystemResourceLocker implements ResourceLocker, Initializable, Finalizable {
   protected readonly logger = getLoggerFor(this);
   private readonly attemptSettings: Required<AttemptSettings>;
+  private readonly lockOptions: LockOptions;
   /** Folder that stores the locks */
   private readonly lockFolder: string;
   private finalized = false;
@@ -69,17 +70,18 @@ export class FileSystemResourceLocker implements ResourceLocker, Initializable,
    * Create a new FileSystemResourceLocker
    * @param args - Configures the locker using the specified FileSystemResourceLockerArgs instance.
    */
-  public constructor(args: FileSystemResourceLockerArgs = {}) {
+  public constructor(args: FileSystemResourceLockerArgs) {
     const { rootFilePath, lockDirectory, attemptSettings } = args;
-    defaultLockOptions.onCompromised = this.customOnCompromised.bind(this);
+    // Need to create lock options for this instance due to the custom `onCompromised`
+    this.lockOptions = { ...defaultLockOptions, onCompromised: this.customOnCompromised.bind(this) };
     this.attemptSettings = { ...attemptDefaults, ...attemptSettings };
-    this.lockFolder = joinFilePath(rootFilePath ?? './', lockDirectory ?? '/.internal/locks');
+    this.lockFolder = joinFilePath(rootFilePath, lockDirectory ?? '/.internal/locks');
   }
 
   /**
    * Wrapper function for all (un)lock operations. Any errors coming from the `fn()` will be swallowed.
    * Only `ENOTACQUIRED` errors wills be thrown (trying to release lock that didn't exist).
-   * This wrapper returns undefined because {@link retryFunction} expects that when a retry needs to happne.s
+   * This wrapper returns undefined because {@link retryFunction} expects that when a retry needs to happen.
    * @param fn - The function reference to swallow errors from.
    * @returns Boolean or undefined.
    */
@@ -101,7 +103,7 @@ export class FileSystemResourceLocker implements ResourceLocker, Initializable,
     const { path } = identifier;
     this.logger.debug(`Acquiring lock for ${path}`);
     try {
-      const opt = this.generateOptions(identifier, defaultLockOptions);
+      const opt = this.generateOptions(identifier, this.lockOptions);
       await retryFunction(
         this.swallowErrors(lock.bind(null, path, opt)),
         this.attemptSettings,

test/unit/util/locking/FileSystemResourceLocker.test.ts

@@ -1,14 +1,19 @@
 import { readdir } from 'fs-extra';
-import { InternalServerError, FileSystemResourceLocker } from '../../../../src';
+import { InternalServerError, FileSystemResourceLocker, joinFilePath } from '../../../../src';
+import { getTestFolder, removeFolder } from '../../../integration/Config';
 
-const lockFolder = './.internal/locks/';
+// Due to the nature of using a file locking library, this is a unit test that writes to disk.
+// In the future ( = if someone has time) we might want to split this up into a unit test with a `proper-lockfile` mock,
+// and an integration test that tests the behaviour of the library.
+const rootFilePath = getTestFolder('FileSystemResourceLocker');
+const lockFolder = joinFilePath(rootFilePath, '.internal/locks/');
 
 describe('A FileSystemResourceLocker', (): void => {
   let locker: FileSystemResourceLocker;
   const identifier = { path: 'http://test.com/foo' };
 
   beforeEach(async(): Promise<void> => {
-    locker = new FileSystemResourceLocker({ attemptSettings: { retryCount: 19, retryDelay: 100 }});
+    locker = new FileSystemResourceLocker({ rootFilePath, attemptSettings: { retryCount: 19, retryDelay: 100 }});
     await locker.initialize();
   });
 
@@ -23,6 +28,7 @@ describe('A FileSystemResourceLocker', (): void => {
 
   afterAll(async(): Promise<void> => {
     await locker.finalize();
+    await removeFolder(rootFilePath);
   });
 
   it('can lock and unlock a resource.', async(): Promise<void> => {
@@ -31,7 +37,7 @@ describe('A FileSystemResourceLocker', (): void => {
   });
 
   it('can lock and unlock a resource with a locker with indefinite retry.', async(): Promise<void> => {
-    const locker2 = new FileSystemResourceLocker({ attemptSettings: { retryCount: -1 }});
+    const locker2 = new FileSystemResourceLocker({ rootFilePath, attemptSettings: { retryCount: -1 }});
     await expect(locker2.acquire(identifier)).resolves.toBeUndefined();
     await expect(locker2.release(identifier)).resolves.toBeUndefined();
     await locker2.finalize();
@@ -116,14 +122,11 @@ describe('A FileSystemResourceLocker', (): void => {
     await expect(readdir(lockFolder)).resolves.toHaveLength(0);
   });
 
-  it('stops proper-lock from throwing errors onCompromise after finalize was called.',
-    async(): Promise<void> => {
+  it('stops proper-lock from throwing errors after finalize was called.', async(): Promise<void> => {
+    // Tests should never access private fields so we need to change this after splitting the test as mentioned above.
+    // Once we have a mock we can check which parameters `unlock` was called with and extract the function from there.
     expect((): void => (locker as any).customOnCompromised(new Error('test'))).toThrow();
     await locker.finalize();
     expect((locker as any).customOnCompromised(new Error('test'))).toBeUndefined();
   });
-
-  it('can create a locker with default AttemptSettings.', async(): Promise<void> => {
-    expect((): FileSystemResourceLocker => new FileSystemResourceLocker()).not.toThrow();
-  });
 });