Merge pull request #7206 from mook-as/path-management/rename-on-write

Path management: rename files in place where possible

.github/actions/spelling/expect.txt

@@ -213,6 +213,7 @@ dustin
 Dwm
 dwmapi
 DWMWA
+EACCESS
 eagerzeroedthick
 eastus
 ebegin
@@ -278,6 +279,7 @@ gabcdef
 gazornaanplatt
 gcs
 GENERALIZEDTIME
+getfattr
 getwindowid
 ghp
 gitmodules
@@ -713,6 +715,7 @@ servernum
 serviceaccount
 servicemonitor
 servicewatcher
+setfattr
 setproxy
 SFC
 sharedscripts

.github/dependabot.yml

@@ -53,6 +53,9 @@ updates:
         # Assume the next release will fix it.
         dependency-name: "unfetch"
         versions: [ "5.0.0" ]
+      - # fs-xattr 0.4.0 later are esm-only.
+        dependency-name: "fs-xattr"
+        versions: [ ">0.3"]
 
   # Maintain dependencies for Golang
   - package-ecosystem: "gomod"

jest.config.js

@@ -10,6 +10,7 @@ module.exports = {
   moduleFileExtensions: [
     'js',
     'json',
+    'node', // For native modules, e.g. fs-xattr
     'ts',
     'vue',
   ],

package.json

@@ -158,6 +158,7 @@
     "nan": "2.20.0",
     "node-gyp": "10.2.0",
     "node-gyp-build": "4.8.1",
+    "node-loader": "^2.0.0",
     "octokit": "3.2.1",
     "ps-tree": "1.2.0",
     "raw-loader": "4.0.2",
@@ -177,7 +178,8 @@
     "string-width": "^4"
   },
   "optionalDependencies": {
-    "dmg-license": "1.0.11"
+    "dmg-license": "1.0.11",
+    "fs-xattr": "0.3.1"
   },
   "browserslist": [
     "node 18",

pkg/rancher-desktop/integrations/manageLinesInFile.ts

@@ -1,80 +1,134 @@
 import fs from 'fs';
-import os from 'os';
 
 import isEqual from 'lodash/isEqual.js';
 
 export const START_LINE = '### MANAGED BY RANCHER DESKTOP START (DO NOT EDIT)';
 export const END_LINE = '### MANAGED BY RANCHER DESKTOP END (DO NOT EDIT)';
 const DEFAULT_FILE_MODE = 0o644;
 
-// Inserts/removes fenced lines into/from a file. Idempotent.
-// @param path The path to the file to work on.
-// @param desiredManagedLines The lines to insert into the file.
-// @param desiredPresent Whether the lines should be present.
+/**
+ * Inserts/removes fenced lines into/from a file. Idempotent.
+ * @param path The path to the file to work on.
+ * @param desiredManagedLines The lines to insert into the file.
+ * @param desiredPresent Whether the lines should be present.
+ */
 export default async function manageLinesInFile(path: string, desiredManagedLines: string[], desiredPresent: boolean): Promise<void> {
-  // read file, creating it if it doesn't exist
-  let currentContent: string;
+  const desired = getDesiredLines(desiredManagedLines, desiredPresent);
+  let fileStats: fs.Stats;
 
   try {
-    currentContent = await fs.promises.readFile(path, 'utf8');
-  } catch (error: any) {
-    if (error.code === 'ENOENT' && desiredPresent) {
-      const lines = buildFileLines([], desiredManagedLines, ['']);
-      const content = lines.join(os.EOL);
+    fileStats = await fs.promises.lstat(path);
+  } catch (ex: any) {
+    if (ex && 'code' in ex && ex.code === 'ENOENT') {
+      // File does not exist.
+      const content = computeTargetContents('', desired);
 
-      await fs.promises.writeFile(path, content, { mode: DEFAULT_FILE_MODE });
+      if (content) {
+        await fs.promises.writeFile(path, content, { mode: DEFAULT_FILE_MODE });
+      }
 
-      return;
-    } else if (error.code === 'ENOENT' && !desiredPresent) {
       return;
     } else {
-      throw error;
+      throw ex;
     }
   }
 
-  // split file into three parts
-  let before: string[];
-  let currentManagedLines: string[];
-  let after: string[];
+  if (fileStats.isFile()) {
+    if (await fileHasExtendedAttributes(path)) {
+      throw new Error(`Refusing to manage ${ path } which has extended attributes`);
+    }
 
-  try {
-    const currentLines = currentContent.split('\n');
+    const tempName = `${ path }.rd-temp`;
 
-    [before, currentManagedLines, after] = splitLinesByDelimiters(currentLines);
-  } catch (error) {
-    throw new Error(`could not split ${ path }: ${ error }`);
-  }
+    await fs.promises.copyFile(path, tempName, fs.constants.COPYFILE_EXCL | fs.constants.COPYFILE_FICLONE);
+
+    try {
+      const currentContents = await fs.promises.readFile(tempName, 'utf-8');
+      const targetContents = computeTargetContents(currentContents, desired);
+
+      if (targetContents === undefined) {
+        // No changes are needed
+        return;
+      }
+
+      if (targetContents === '') {
+        // The resulting file is empty; unlink it.
+        await fs.promises.unlink(path);
+
+        return;
+      }
 
-  // make the changes
-  if (desiredPresent && !isEqual(currentManagedLines, desiredManagedLines)) {
-    // This is needed to ensure the file ends with an EOL
-    if (after.length === 0) {
-      after = [''];
+      await fs.promises.writeFile(tempName, targetContents, 'utf-8');
+      await fs.promises.rename(tempName, path);
+    } finally {
+      try {
+        await fs.promises.unlink(tempName);
+      } catch {
+        // Ignore errors unlinking the temporary file; if everything went well,
+        // it no longer exists anyway.
+      }
     }
-    const newLines = buildFileLines(before, desiredManagedLines, after);
-    const newContent = newLines.join(os.EOL);
+  } else if (fileStats.isSymbolicLink()) {
+    const backupPath = `${ path }.rd-backup~`;
 
-    await fs.promises.writeFile(path, newContent);
-  }
-  if (!desiredPresent) {
-    // Ignore the extra empty line that came from the managed block.
-    if (after.length === 1 && after[0] === '') {
-      after = [];
+    await fs.promises.copyFile(path, backupPath, fs.constants.COPYFILE_EXCL | fs.constants.COPYFILE_FICLONE);
+
+    const currentContents = await fs.promises.readFile(backupPath, 'utf-8');
+    const targetContents = computeTargetContents(currentContents, desired);
+
+    if (targetContents === undefined) {
+      // No changes are needed; just remove the backup file again.
+      await fs.promises.unlink(backupPath);
+
+      return;
     }
-    if (before.length === 0 && after.length === 0) {
-      await fs.promises.rm(path);
-    } else {
-      const newLines = buildFileLines(before, [], after);
-      const newContent = newLines.join(os.EOL);
+    // Always write the file, even if the result will be empty.
+    await fs.promises.writeFile(path, targetContents, 'utf-8');
+
+    const actualContents = await fs.promises.readFile(path, 'utf-8');
 
-      await fs.promises.writeFile(path, newContent);
+    if (!isEqual(targetContents, actualContents)) {
+      throw new Error(`Error writing to ${ path }: written contents are unexpected; see backup in ${ backupPath }`);
     }
+    await fs.promises.unlink(backupPath);
+  } else {
+    // Target exists, and is neither a normal file nor a symbolic link.
+    // Return with an error.
+    throw new Error(`Refusing to manage ${ path } which is neither a regular file nor a symbolic link`);
   }
 }
 
-// Splits a file into three arrays containing the lines before the managed portion,
-// the lines in the managed portion and the lines after the managed portion.
-// @param lines An array where each element represents a line in a file.
+/**
+ * Check if the given file has any extended attributes.
+ *
+ * We do this check because we are not confident of being able to write the file
+ * atomically (that is, either the old content or new content is visible) while
+ * also preserving extended attributes.
+ */
+async function fileHasExtendedAttributes(filePath: string): Promise<boolean> {
+  try {
+    // eslint-disable-next-line @typescript-eslint/ban-ts-comment -- This only fails on Windows
+    // @ts-ignore // fs-xattr is not available on Windows
+    const { list } = await import('fs-xattr');
+
+    return (await list(filePath)).length > 0;
+  } catch {
+    if (process.env.NODE_ENV === 'test' && process.env.RD_TEST !== 'e2e') {
+      // When running unit tests, assume they do not have extended attributes.
+      return false;
+    }
+
+    console.error(`Failed to import fs-xattr, cannot check for extended attributes on ${ filePath }; assuming it exists.`);
+
+    return true;
+  }
+}
+
+/**
+ * Splits a file into three arrays containing the lines before the managed portion,
+ * the lines in the managed portion and the lines after the managed portion.
+ * @param lines An array where each element represents a line in a file.
+ */
 function splitLinesByDelimiters(lines: string[]): [string[], string[], string[]] {
   const startIndex = lines.indexOf(START_LINE);
   const endIndex = lines.indexOf(END_LINE);
@@ -94,12 +148,47 @@ function splitLinesByDelimiters(lines: string[]): [string[], string[], string[]]
   return [before, currentManagedLines, after];
 }
 
-// Builds an array where each element represents a line in a file.
-// @param before The portion of the file before the managed lines.
-// @param toInsert The managed lines, not including the fences.
-// @param after The portion of the file after the managed lines.
-function buildFileLines(before: string[], toInsert: string[], after: string[]): string[] {
-  const rancherDesktopLines = toInsert.length > 0 ? [START_LINE, ...toInsert, END_LINE] : [];
+/**
+ * Calculate the desired content of the managed lines.
+ * @param desiredManagedLines The lines to insert into the file.
+ * @param desiredPresent Whether the lines should be present.
+ * @returns The lines that should end up in the managed section of the final file.
+ */
+function getDesiredLines(desiredManagedLines: string[], desiredPresent: boolean): string[] {
+  const desired = desiredPresent && desiredManagedLines.length > 0;
+
+  return desired ? [START_LINE, ...desiredManagedLines, END_LINE] : [];
+}
+
+/**
+ * Given the current contents of the file, determine what the final file
+ * contents should be.
+ * @param currentContents The current contents of the file.
+ * @param desired The desired content of the managed lines.
+ * @returns The final content; if no changes are needed, `undefined` is returned.
+ *          There will never be any leading empty lines,
+ *          and there will always be exactly one trailing empty line.
+ */
+function computeTargetContents(currentContents: string, desired: string[]): string | undefined {
+  const [before, current, after] = splitLinesByDelimiters(currentContents.split('\n'));
+
+  if (isEqual(current, desired)) {
+    // No changes are needed
+    return undefined;
+  }
+
+  const lines = [...before, ...desired, ...after];
+
+  // Remove all leading empty lines.
+  while (lines.length > 0 && lines[0] === '') {
+    lines.shift();
+  }
+  // Remove all trailing empty lines.
+  while (lines.length > 0 && lines[lines.length - 1] === '') {
+    lines.pop();
+  }
+  // Add one trailing empty line to the end.
+  lines.push('');
 
-  return [...before, ...rancherDesktopLines, ...after];
+  return lines.join('\n');
 }

pkg/rancher-desktop/utils/testUtils/mockResources.ts

@@ -0,0 +1,21 @@
+// `Symbol.dispose` exists as of NodeJS 20; if it's unset, set it (because we
+// are currently on NodeJS 18).
+(Symbol as any).dispose ??= Symbol.for('nodejs.dispose');
+
+/**
+ * Given a Jest SpyInstance, return it as a Disposable such that mockRestore will
+ * be called when the instance goes out of scope.
+ * @note This will no longer be needed as of Jest 30 (where it's built in).
+ */
+export function withResource<
+  T = any,
+  Y extends any[] = any,
+  C = any,
+  U extends jest.MockInstance<T, Y, C> = any,
+>(input: U): U & Disposable {
+  (input as any)[Symbol.dispose] = () => {
+    input.mockRestore();
+  };
+
+  return input as U & Disposable;
+}

scripts/lib/build-utils.ts

@@ -133,7 +133,7 @@ export default {
       devtool:   this.isDevelopment ? 'source-map' : false,
       resolve:   {
         alias:      { '@pkg': path.resolve(this.rootDir, 'pkg', 'rancher-desktop') },
-        extensions: ['.ts', '.js', '.json'],
+        extensions: ['.ts', '.js', '.json', '.node'],
         modules:    ['node_modules'],
       },
       output: {
@@ -166,6 +166,10 @@ export default {
             exclude: [/(?:^|[/\\])assets[/\\]scripts[/\\]/, this.distDir],
             use:     { loader: 'js-yaml-loader' },
           },
+          {
+            test: /\.node$/,
+            use:  { loader: 'node-loader' },
+          },
           {
             test: /(?:^|[/\\])assets[/\\]scripts[/\\]/,
             use:  { loader: 'raw-loader' },