Skip to content

Commit

Permalink
Path management: Copy extended attributes
Browse files Browse the repository at this point in the history
It looks like the incidence of extended attributes on files we want to
manage is higher than expected; instead of bailing on any extended
attributes, try to copy them instead.

Signed-off-by: Mark Yen <mark.yen@suse.com>
  • Loading branch information
mook-as committed Sep 3, 2024
1 parent 6cb95f6 commit 9cea974
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 22 deletions.
39 changes: 19 additions & 20 deletions pkg/rancher-desktop/integrations/manageLinesInFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,11 @@ function newErrorWithPath<T extends Record<string, any>>(messageTemplate: (input
export const ErrorDeterminingExtendedAttributes =
newErrorWithPath(({ path }: {path: string}) => `Failed to determine if \`${ path }\` contains extended attributes`);
/**
* `ErrorHasExtendedAttributes` signifies that we were unable to process a file
* because it has extended attributes that would not have been preserved had we
* tried to edit it.
* `ErrorCopyingExtendedAttributes occurs if we failed to copy extended
* attributes while managing a file.
*/
export const ErrorHasExtendedAttributes =
newErrorWithPath(({ path }: {path: string}) => `Refusing to manage \`${ path }\` which has extended attributes`);
export const ErrorCopyingExtendedAttributes =
newErrorWithPath(({ path }: {path: string}) => `Failed to copy extended attributes while managing \`${ path }\``);
/**
* `ErrorNotRegularFile` signifies that we were unable to process a file because
* it is not a regular file (e.g. a named pipe or a device).
Expand Down Expand Up @@ -87,10 +86,6 @@ export default async function manageLinesInFile(path: string, desiredManagedLine
}

if (fileStats.isFile()) {
if (await fileHasExtendedAttributes(path)) {
throw new ErrorHasExtendedAttributes({ path });
}

const tempName = `${ path }.rd-temp`;

await fs.promises.copyFile(path, tempName, fs.constants.COPYFILE_EXCL | fs.constants.COPYFILE_FICLONE);
Expand All @@ -111,6 +106,7 @@ export default async function manageLinesInFile(path: string, desiredManagedLine
return;
}

await copyFileExtendedAttributes(path, tempName);
await fs.promises.writeFile(tempName, targetContents, 'utf-8');
await fs.promises.rename(tempName, path);
} finally {
Expand Down Expand Up @@ -152,28 +148,31 @@ export default async function manageLinesInFile(path: string, desiredManagedLine
}

/**
* 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.
* Copies extended attributes from an existing file to a different file. Both
* files must already exist.
*/
async function fileHasExtendedAttributes(filePath: string): Promise<boolean> {
async function copyFileExtendedAttributes(fromPath: string, toPath: string): Promise<void> {
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');
const { list, get, set } = await import('fs-xattr');

for (const attr of await list(fromPath)) {
const value = await get(fromPath, attr);

return (await list(filePath)).length > 0;
await set(toPath, attr, value);
}
} catch (cause) {
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 }:`, cause);
if (cause && typeof cause === 'object' && 'code' in cause && cause.code === 'MODULE_NOT_FOUND') {
console.error(`Failed to import fs-xattr, cannot copy extended attributes from ${ fromPath }:`, cause);

throw new ErrorDeterminingExtendedAttributes({ path: filePath }, { cause });
throw new ErrorDeterminingExtendedAttributes({ path: fromPath }, { cause });
}
throw new ErrorCopyingExtendedAttributes({ path: fromPath }, { cause });
}
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/rancher-desktop/main/diagnostics/pathManagement.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { DiagnosticsCategory, DiagnosticsChecker, DiagnosticsCheckerResult, DiagnosticsCheckerSingleResult } from './types';

import { ErrorDeterminingExtendedAttributes, ErrorHasExtendedAttributes, ErrorNotRegularFile, ErrorWritingFile } from '@pkg/integrations/manageLinesInFile';
import { ErrorDeterminingExtendedAttributes, ErrorCopyingExtendedAttributes, ErrorNotRegularFile, ErrorWritingFile } from '@pkg/integrations/manageLinesInFile';
import mainEvents from '@pkg/main/mainEvents';

const cachedResults: Record<string, DiagnosticsCheckerResult> = {};
Expand Down Expand Up @@ -41,7 +41,7 @@ mainEvents.on('diagnostics-event', (id, state) => {
return { passed: true, description: `\`${ fileName }\` is managed` };
}

if (error instanceof ErrorHasExtendedAttributes) {
if (error instanceof ErrorCopyingExtendedAttributes) {
return { fixes: [{ description: `Remove extended attributes from \`${ fileName }\`` }] };
}

Expand Down

0 comments on commit 9cea974

Please sign in to comment.