fix(server): dont follow symlinks (#32614)
(cherry picked from commit 10e216b8385ac40e6aa150f75fc4730c4e8a7494)
This commit is contained in:
committed by
Codex
parent
ec3359f1cb
commit
42dffaa0c8
@@ -1,4 +1,4 @@
|
|||||||
import mockfs from 'mock-fs';
|
import mockfs, { symlink } from 'mock-fs';
|
||||||
import { vitest } from 'vitest';
|
import { vitest } from 'vitest';
|
||||||
import { CrawlOptionsDto } from 'src/dtos/library.dto.js';
|
import { CrawlOptionsDto } from 'src/dtos/library.dto.js';
|
||||||
import { LoggingRepository } from 'src/repositories/logging.repository.js';
|
import { LoggingRepository } from 'src/repositories/logging.repository.js';
|
||||||
@@ -216,6 +216,25 @@ describe(StorageRepository.name, () => {
|
|||||||
mockfs.restore();
|
mockfs.restore();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it.each(['crawl', 'walk'] as const)('%s should skip symbolic links to files and directories', async (method) => {
|
||||||
|
mockfs({
|
||||||
|
'/photos/image.jpg': '',
|
||||||
|
'/photos/subfolder/image.jpg': '',
|
||||||
|
'/outside/secret.txt': 'secret',
|
||||||
|
'/outside/album/image.jpg': '',
|
||||||
|
'/photos/file-link.jpg': symlink({ path: '/outside/secret.txt' }),
|
||||||
|
'/photos/directory-link': symlink({ path: '/outside/album' }),
|
||||||
|
'/photos/internal-file-link.jpg': symlink({ path: '/photos/image.jpg' }),
|
||||||
|
'/photos/internal-directory-link': symlink({ path: '/photos/subfolder' }),
|
||||||
|
'/photos/broken-link.jpg': symlink({ path: '/outside/missing.jpg' }),
|
||||||
|
});
|
||||||
|
|
||||||
|
const options = { pathsToCrawl: ['/photos'], take: 1 };
|
||||||
|
const batches = method === 'crawl' ? [await sut.crawl(options)] : await Array.fromAsync(sut.walk(options));
|
||||||
|
|
||||||
|
expect(batches.flat().toSorted()).toEqual(['/photos/image.jpg', '/photos/subfolder/image.jpg']);
|
||||||
|
});
|
||||||
|
|
||||||
describe('crawl', () => {
|
describe('crawl', () => {
|
||||||
for (const { test, options, files } of tests) {
|
for (const { test, options, files } of tests) {
|
||||||
it(test, async () => {
|
it(test, async () => {
|
||||||
@@ -240,7 +259,10 @@ describe(StorageRepository.name, () => {
|
|||||||
const onError = vitest.fn();
|
const onError = vitest.fn();
|
||||||
const close = sut.watch(['/photos'], {}, { onAdd, onChange, onError, onReady, onUnlink });
|
const close = sut.watch(['/photos'], {}, { onAdd, onChange, onError, onReady, onUnlink });
|
||||||
|
|
||||||
expect(mocks.watch).toHaveBeenCalledWith(['/photos'], expect.objectContaining({ ignored: expect.any(Function) }));
|
expect(mocks.watch).toHaveBeenCalledWith(
|
||||||
|
['/photos'],
|
||||||
|
expect.objectContaining({ followSymlinks: false, ignored: expect.any(Function) }),
|
||||||
|
);
|
||||||
|
|
||||||
const error = new Error('watch error');
|
const error = new Error('watch error');
|
||||||
getHandler('ready')();
|
getHandler('ready')();
|
||||||
|
|||||||
@@ -237,6 +237,7 @@ export class StorageRepository {
|
|||||||
absolute: true,
|
absolute: true,
|
||||||
caseSensitiveMatch: false,
|
caseSensitiveMatch: false,
|
||||||
onlyFiles: true,
|
onlyFiles: true,
|
||||||
|
followSymbolicLinks: false,
|
||||||
dot: includeHidden,
|
dot: includeHidden,
|
||||||
ignore: exclusionPatterns,
|
ignore: exclusionPatterns,
|
||||||
});
|
});
|
||||||
@@ -256,6 +257,7 @@ export class StorageRepository {
|
|||||||
absolute: true,
|
absolute: true,
|
||||||
caseSensitiveMatch: false,
|
caseSensitiveMatch: false,
|
||||||
onlyFiles: true,
|
onlyFiles: true,
|
||||||
|
followSymbolicLinks: false,
|
||||||
dot: includeHidden,
|
dot: includeHidden,
|
||||||
ignore: exclusionPatterns,
|
ignore: exclusionPatterns,
|
||||||
});
|
});
|
||||||
@@ -284,7 +286,12 @@ export class StorageRepository {
|
|||||||
strictSlashes: false,
|
strictSlashes: false,
|
||||||
});
|
});
|
||||||
|
|
||||||
const watcher = chokidarWatch(paths, { ...options, ignored: (path) => matchesIgnoredPath(path) });
|
const watcher = chokidarWatch(paths, {
|
||||||
|
...options,
|
||||||
|
followSymlinks: false,
|
||||||
|
// Chokidar still emits events for symlinks when followSymlinks is false.
|
||||||
|
ignored: (path, stats) => !!stats?.isSymbolicLink() || matchesIgnoredPath(path),
|
||||||
|
});
|
||||||
|
|
||||||
watcher.on('ready', () => events.onReady?.());
|
watcher.on('ready', () => events.onReady?.());
|
||||||
watcher.on('add', (path) => events.onAdd?.(path));
|
watcher.on('add', (path) => events.onAdd?.(path));
|
||||||
|
|||||||
@@ -1,6 +1,8 @@
|
|||||||
import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises';
|
import { mkdir, mkdtemp, rename, rm, symlink, writeFile } from 'node:fs/promises';
|
||||||
import { tmpdir } from 'node:os';
|
import { tmpdir } from 'node:os';
|
||||||
import { join } from 'node:path';
|
import { join } from 'node:path';
|
||||||
|
import { setTimeout } from 'node:timers/promises';
|
||||||
|
import { vitest } from 'vitest';
|
||||||
import { LoggingRepository } from 'src/repositories/logging.repository.js';
|
import { LoggingRepository } from 'src/repositories/logging.repository.js';
|
||||||
import { StorageRepository, type WatchOptions } from 'src/repositories/storage.repository.js';
|
import { StorageRepository, type WatchOptions } from 'src/repositories/storage.repository.js';
|
||||||
import { automock } from 'test/utils.js';
|
import { automock } from 'test/utils.js';
|
||||||
@@ -32,6 +34,8 @@ describe(StorageRepository.name, () => {
|
|||||||
|
|
||||||
const startWatching = (options: WatchOptions) => {
|
const startWatching = (options: WatchOptions) => {
|
||||||
const paths: string[] = [];
|
const paths: string[] = [];
|
||||||
|
const changedPaths: string[] = [];
|
||||||
|
const unlinkedPaths: string[] = [];
|
||||||
let resolveReady!: () => void;
|
let resolveReady!: () => void;
|
||||||
|
|
||||||
const ready = new Promise<void>((resolve) => (resolveReady = resolve));
|
const ready = new Promise<void>((resolve) => (resolveReady = resolve));
|
||||||
@@ -39,10 +43,16 @@ describe(StorageRepository.name, () => {
|
|||||||
onAdd: (path) => {
|
onAdd: (path) => {
|
||||||
paths.push(path);
|
paths.push(path);
|
||||||
},
|
},
|
||||||
|
onChange: (path) => {
|
||||||
|
changedPaths.push(path);
|
||||||
|
},
|
||||||
|
onUnlink: (path) => {
|
||||||
|
unlinkedPaths.push(path);
|
||||||
|
},
|
||||||
onReady: resolveReady,
|
onReady: resolveReady,
|
||||||
});
|
});
|
||||||
|
|
||||||
return { close, paths, ready };
|
return { close, paths, changedPaths, unlinkedPaths, ready };
|
||||||
};
|
};
|
||||||
|
|
||||||
it('should emit add events for initial files', async () => {
|
it('should emit add events for initial files', async () => {
|
||||||
@@ -69,6 +79,92 @@ describe(StorageRepository.name, () => {
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should ignore symbolic links to files and directories', async () => {
|
||||||
|
const outsideDir = await mkdtemp(join(tmpdir(), 'immich-storage-outside-'));
|
||||||
|
|
||||||
|
try {
|
||||||
|
await writeFile(join(outsideDir, 'secret.txt'), 'secret');
|
||||||
|
await writeFile(join(outsideDir, 'photo.jpg'), 'outside');
|
||||||
|
await symlink(join(outsideDir, 'secret.txt'), join(tempDir, 'file-link.jpg'));
|
||||||
|
await symlink(outsideDir, join(tempDir, 'directory-link'));
|
||||||
|
await symlink(includedFile, join(tempDir, 'internal-file-link.jpg'));
|
||||||
|
await symlink(join(tempDir, 'included'), join(tempDir, 'internal-directory-link'));
|
||||||
|
|
||||||
|
const watcher = startWatching({ ignoreInitial: false });
|
||||||
|
|
||||||
|
try {
|
||||||
|
await watcher.ready;
|
||||||
|
|
||||||
|
expect(watcher.paths.toSorted()).toEqual([includedFile, excludedFile].toSorted());
|
||||||
|
} finally {
|
||||||
|
await watcher.close();
|
||||||
|
}
|
||||||
|
} finally {
|
||||||
|
await rm(outsideDir, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it.each([
|
||||||
|
{ type: 'file', action: 'created' },
|
||||||
|
{ type: 'directory', action: 'created' },
|
||||||
|
{ type: 'file', action: 'retargeted' },
|
||||||
|
{ type: 'directory', action: 'retargeted' },
|
||||||
|
])('should ignore $type symlinks $action after the watcher is ready', async ({ type, action }) => {
|
||||||
|
const outsideDir = await mkdtemp(join(tmpdir(), 'immich-storage-outside-'));
|
||||||
|
|
||||||
|
try {
|
||||||
|
const targetDir = join(outsideDir, 'target');
|
||||||
|
const targetFile = join(targetDir, 'photo.jpg');
|
||||||
|
await mkdir(targetDir);
|
||||||
|
await writeFile(targetFile, 'outside');
|
||||||
|
|
||||||
|
const link = join(tempDir, type === 'file' ? 'link.jpg' : 'link');
|
||||||
|
const target = type === 'file' ? targetFile : targetDir;
|
||||||
|
if (action === 'retargeted') {
|
||||||
|
await symlink(type === 'file' ? includedFile : join(tempDir, 'included'), link);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Use real Chokidar events without relying on platform-specific native watcher limits.
|
||||||
|
const watcher = startWatching({ ignoreInitial: true, usePolling: true, interval: 50 });
|
||||||
|
|
||||||
|
try {
|
||||||
|
await watcher.ready;
|
||||||
|
|
||||||
|
if (action === 'retargeted') {
|
||||||
|
const replacementLink = join(outsideDir, 'replacement-link');
|
||||||
|
await symlink(target, replacementLink);
|
||||||
|
await rename(replacementLink, link);
|
||||||
|
} else {
|
||||||
|
await symlink(target, link);
|
||||||
|
}
|
||||||
|
// Confirm the watcher processes real file events before checking for unwanted symlink events.
|
||||||
|
const newFile = join(tempDir, 'new-photo.jpg');
|
||||||
|
await writeFile(newFile, 'new');
|
||||||
|
await writeFile(includedFile, 'changed inside');
|
||||||
|
await vitest.waitFor(
|
||||||
|
() => {
|
||||||
|
expect(watcher.paths).toContain(newFile);
|
||||||
|
expect(watcher.changedPaths).toContain(includedFile);
|
||||||
|
},
|
||||||
|
{ timeout: 3000 },
|
||||||
|
);
|
||||||
|
|
||||||
|
await writeFile(targetFile, 'changed outside');
|
||||||
|
|
||||||
|
// Allow deferred filesystem events and Chokidar's directory rescan throttle to settle.
|
||||||
|
await setTimeout(1500);
|
||||||
|
|
||||||
|
expect(watcher.paths).toEqual([newFile]);
|
||||||
|
expect(watcher.changedPaths).toEqual([includedFile]);
|
||||||
|
expect(watcher.unlinkedPaths).toEqual([]);
|
||||||
|
} finally {
|
||||||
|
await watcher.close();
|
||||||
|
}
|
||||||
|
} finally {
|
||||||
|
await rm(outsideDir, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
it.each(annoyingExclusionPatterns)('should ignore folders with %s in their name', async (char) => {
|
it.each(annoyingExclusionPatterns)('should ignore folders with %s in their name', async (char) => {
|
||||||
const ignoredFolder = `${char}folder`;
|
const ignoredFolder = `${char}folder`;
|
||||||
const ignoredFile = join(tempDir, ignoredFolder, 'photo.jpg');
|
const ignoredFile = join(tempDir, ignoredFolder, 'photo.jpg');
|
||||||
|
|||||||
Reference in New Issue
Block a user