diff --git a/server/src/repositories/storage.repository.spec.ts b/server/src/repositories/storage.repository.spec.ts index 3d1e503ed..dc10e2a66 100644 --- a/server/src/repositories/storage.repository.spec.ts +++ b/server/src/repositories/storage.repository.spec.ts @@ -1,4 +1,4 @@ -import mockfs from 'mock-fs'; +import mockfs, { symlink } from 'mock-fs'; import { vitest } from 'vitest'; import { CrawlOptionsDto } from 'src/dtos/library.dto.js'; import { LoggingRepository } from 'src/repositories/logging.repository.js'; @@ -216,6 +216,25 @@ describe(StorageRepository.name, () => { 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', () => { for (const { test, options, files } of tests) { it(test, async () => { @@ -240,7 +259,10 @@ describe(StorageRepository.name, () => { const onError = vitest.fn(); 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'); getHandler('ready')(); diff --git a/server/src/repositories/storage.repository.ts b/server/src/repositories/storage.repository.ts index 43766e91f..ef8800091 100644 --- a/server/src/repositories/storage.repository.ts +++ b/server/src/repositories/storage.repository.ts @@ -237,6 +237,7 @@ export class StorageRepository { absolute: true, caseSensitiveMatch: false, onlyFiles: true, + followSymbolicLinks: false, dot: includeHidden, ignore: exclusionPatterns, }); @@ -256,6 +257,7 @@ export class StorageRepository { absolute: true, caseSensitiveMatch: false, onlyFiles: true, + followSymbolicLinks: false, dot: includeHidden, ignore: exclusionPatterns, }); @@ -284,7 +286,12 @@ export class StorageRepository { 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('add', (path) => events.onAdd?.(path)); diff --git a/server/test/medium/specs/repositories/storage.repository.spec.ts b/server/test/medium/specs/repositories/storage.repository.spec.ts index bf34c5c25..a25e761b4 100644 --- a/server/test/medium/specs/repositories/storage.repository.spec.ts +++ b/server/test/medium/specs/repositories/storage.repository.spec.ts @@ -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 { join } from 'node:path'; +import { setTimeout } from 'node:timers/promises'; +import { vitest } from 'vitest'; import { LoggingRepository } from 'src/repositories/logging.repository.js'; import { StorageRepository, type WatchOptions } from 'src/repositories/storage.repository.js'; import { automock } from 'test/utils.js'; @@ -32,6 +34,8 @@ describe(StorageRepository.name, () => { const startWatching = (options: WatchOptions) => { const paths: string[] = []; + const changedPaths: string[] = []; + const unlinkedPaths: string[] = []; let resolveReady!: () => void; const ready = new Promise((resolve) => (resolveReady = resolve)); @@ -39,10 +43,16 @@ describe(StorageRepository.name, () => { onAdd: (path) => { paths.push(path); }, + onChange: (path) => { + changedPaths.push(path); + }, + onUnlink: (path) => { + unlinkedPaths.push(path); + }, onReady: resolveReady, }); - return { close, paths, ready }; + return { close, paths, changedPaths, unlinkedPaths, ready }; }; 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) => { const ignoredFolder = `${char}folder`; const ignoredFile = join(tempDir, ignoredFolder, 'photo.jpg');