Added proper file locking to FileSaltStore

Author: cmraible

package.json

@@ -84,6 +84,7 @@
     "fastify": "5.6.1",
     "fastify-plugin": "5.1.0",
     "pino": "9.7.0",
+    "proper-lockfile": "4.1.2",
     "ua-parser-js": "1.0.41"
   }
 }

src/services/salt-store/FileSaltStore.ts

@@ -1,5 +1,6 @@
 import {promises as fs} from 'fs';
 import * as path from 'path';
+import * as Lockfile from 'proper-lockfile';
 import {ISaltStore, SaltRecord} from './ISaltStore';
 import logger from '../../utils/logger';
 
@@ -112,10 +113,33 @@ export class FileSaltStore implements ISaltStore {
 
     /**
      * Executes a file operation with queuing to prevent concurrent writes.
+     * Uses both per-instance queuing and cross-instance file locking.
      */
     private async executeFileOperation<T>(operation: () => Promise<T>): Promise<T> {
-        // Queue operations to prevent concurrent file access issues
-        const currentOperation = this.fileOperationPromise.then(operation);
+        // Queue operations to prevent concurrent file access issues within this instance
+        const currentOperation = this.fileOperationPromise.then(async () => {
+            // Acquire cross-instance lock before accessing file
+            let release: (() => Promise<void>) | undefined;
+            try {
+                release = await Lockfile.lock(this.filePath, {
+                    retries: {
+                        retries: 10,
+                        minTimeout: 50,
+                        maxTimeout: 500
+                    },
+                    stale: 10000, // Consider lock stale after 10s
+                    realpath: false // Don't resolve symlinks
+                });
+
+                // Execute operation while holding lock
+                return await operation();
+            } finally {
+                // Always release lock
+                if (release) {
+                    await release();
+                }
+            }
+        });
         this.fileOperationPromise = currentOperation.then(() => {}, () => {});
         return currentOperation;
     }

src/types/proper-lockfile.d.ts

@@ -0,0 +1,27 @@
+declare module 'proper-lockfile' {
+    export interface LockOptions {
+        retries?: {
+            retries?: number;
+            minTimeout?: number;
+            maxTimeout?: number;
+            factor?: number;
+            randomize?: boolean;
+        };
+        stale?: number;
+        realpath?: boolean;
+        fs?: Record<string, unknown>;
+        onCompromised?: (err: Error) => void;
+    }
+
+    export interface UnlockOptions {
+        realpath?: boolean;
+        fs?: Record<string, unknown>;
+    }
+
+    export function lock(file: string, options?: LockOptions): Promise<() => Promise<void>>;
+    export function unlock(file: string, options?: UnlockOptions): Promise<void>;
+    export function check(file: string, options?: LockOptions): Promise<boolean>;
+    export function lockSync(file: string, options?: LockOptions): () => void;
+    export function unlockSync(file: string, options?: UnlockOptions): void;
+    export function checkSync(file: string, options?: LockOptions): boolean;
+}

test/unit/services/salt-store/FileSaltStore.test.ts

@@ -15,12 +15,17 @@ describe('FileSaltStore', () => {
     });
 
     afterEach(async () => {
-        // Clean up test file
+        // Clean up test file and lock directory
         try {
             await fs.unlink(testFilePath);
         } catch {
             // Ignore errors if file doesn't exist
         }
+        try {
+            await fs.rm(`${testFilePath}.lock`, {recursive: true, force: true});
+        } catch {
+            // Ignore errors if lock directory doesn't exist
+        }
     });
 
     describe('set', () => {
@@ -542,6 +547,7 @@ describe('FileSaltStore', () => {
 
             // Clean up
             await fs.rm(path.dirname(nestedPath), {recursive: true, force: true});
+            await fs.rm(`${nestedPath}.lock`, {recursive: true, force: true}).catch(() => {});
         });
 
         it('should handle concurrent writes correctly', async () => {
@@ -630,6 +636,8 @@ describe('FileSaltStore', () => {
             // Clean up
             await fs.unlink(store1Path);
             await fs.unlink(store2Path);
+            await fs.rm(`${store1Path}.lock`, {recursive: true, force: true}).catch(() => {});
+            await fs.rm(`${store2Path}.lock`, {recursive: true, force: true}).catch(() => {});
         });
 
         it('should work correctly with multiple instances using same file', async () => {

yarn.lock

@@ -5148,7 +5148,7 @@ gopd@^1.0.1, gopd@^1.2.0:
   resolved "https://registry.yarnpkg.com/gopd/-/gopd-1.2.0.tgz#89f56b8217bdbc8802bd299df6d7f1081d7e51a1"
   integrity sha512-ZUKRh6/kUFoAiTAtTYPZJ3hw9wNxx+BIBOijnlG9PnrJsCcSjs1wyyD6vJpaYtgnzDrKYRSqf3OO6Rfa93xsRg==
 
-graceful-fs@^4.1.6, graceful-fs@^4.2.0:
+graceful-fs@^4.1.6, graceful-fs@^4.2.0, graceful-fs@^4.2.4:
   version "4.2.11"
   resolved "https://registry.yarnpkg.com/graceful-fs/-/graceful-fs-4.2.11.tgz#4183e4e8bf08bb6e05bbb2f7d2e0c8f712ca40e3"
   integrity sha512-RbJ5/jmFcNNCcDV5o9eTnBLJ/HszWV0P73bc+Ff4nS/rJj+YaS6IGyiOL0VoBYX+l1Wrl3k63h/KrH+nhJ0XvQ==
@@ -6489,6 +6489,15 @@ promise-map-series@^0.2.1:
   dependencies:
     rsvp "^3.0.14"
 
+[email protected]:
+  version "4.1.2"
+  resolved "https://registry.yarnpkg.com/proper-lockfile/-/proper-lockfile-4.1.2.tgz#c8b9de2af6b2f1601067f98e01ac66baa223141f"
+  integrity sha512-TjNPblN4BwAWMXU8s9AEz4JmQxnD1NNL7bNOY/AKUzyamc379FWASUhc/K1pL2noVb+XmZKLL68cjzLsiOAMaA==
+  dependencies:
+    graceful-fs "^4.2.4"
+    retry "^0.12.0"
+    signal-exit "^3.0.2"
+
 proto3-json-serializer@^2.0.2:
   version "2.0.2"
   resolved "https://registry.yarnpkg.com/proto3-json-serializer/-/proto3-json-serializer-2.0.2.tgz#5b705203b4d58f3880596c95fad64902617529dd"
@@ -6728,6 +6737,11 @@ retry-request@^8.0.0:
     extend "^3.0.2"
     teeny-request "^10.0.0"
 
+retry@^0.12.0:
+  version "0.12.0"
+  resolved "https://registry.yarnpkg.com/retry/-/retry-0.12.0.tgz#1b42a6266a21f07421d1b0b54b7dc167b01c013b"
+  integrity sha512-9LkiTwjUh6rT555DtE9rTX+BKByPfrMzEAtnlEtdEwr3Nkffwiihqe2bWADg+OQRjt9gl6ICdmB/ZFDCGAtSow==
+
 reusify@^1.0.4:
   version "1.1.0"
   resolved "https://registry.yarnpkg.com/reusify/-/reusify-1.1.0.tgz#0fe13b9522e1473f51b558ee796e08f11f9b489f"