fix: only check external path once (#4419)

server/src/domain/library/library.service.spec.ts

@@ -415,61 +415,6 @@ describe(LibraryService.name, () => {
       });
     });
 
-    it('should skip an asset if the user cannot be found', async () => {
-      userMock.get.mockResolvedValue(null);
-
-      const mockLibraryJob: ILibraryFileJob = {
-        id: libraryStub.externalLibrary1.id,
-        ownerId: mockUser.id,
-        assetPath: '/data/user1/photo.jpg',
-        force: false,
-      };
-
-      expect(sut.handleAssetRefresh(mockLibraryJob)).resolves.toBe(false);
-    });
-
-    it('should skip an asset if external path is not set', async () => {
-      mockUser = userStub.admin;
-      userMock.get.mockResolvedValue(mockUser);
-
-      const mockLibraryJob: ILibraryFileJob = {
-        id: libraryStub.externalLibrary1.id,
-        ownerId: mockUser.id,
-        assetPath: '/data/user1/photo.jpg',
-        force: false,
-      };
-
-      expect(sut.handleAssetRefresh(mockLibraryJob)).resolves.toBe(false);
-    });
-
-    it("should skip an asset if it isn't in the external path", async () => {
-      mockUser = userStub.externalPath1;
-      userMock.get.mockResolvedValue(mockUser);
-
-      const mockLibraryJob: ILibraryFileJob = {
-        id: libraryStub.externalLibrary1.id,
-        ownerId: mockUser.id,
-        assetPath: '/etc/rootpassword.jpg',
-        force: false,
-      };
-
-      expect(sut.handleAssetRefresh(mockLibraryJob)).resolves.toBe(false);
-    });
-
-    it('should skip an asset if directory traversal is attempted', async () => {
-      mockUser = userStub.externalPath1;
-      userMock.get.mockResolvedValue(mockUser);
-
-      const mockLibraryJob: ILibraryFileJob = {
-        id: libraryStub.externalLibrary1.id,
-        ownerId: mockUser.id,
-        assetPath: '/data/user1/../../etc/rootpassword.jpg',
-        force: false,
-      };
-
-      expect(sut.handleAssetRefresh(mockLibraryJob)).resolves.toBe(false);
-    });
-
     it('should set a missing asset to offline', async () => {
       storageMock.stat.mockRejectedValue(new Error());
 

server/src/domain/library/library.service.ts

@@ -156,17 +156,6 @@ export class LibraryService {
   async handleAssetRefresh(job: ILibraryFileJob) {
     const assetPath = path.normalize(job.assetPath);
 
-    const user = await this.userRepository.get(job.ownerId);
-    if (!user?.externalPath) {
-      this.logger.warn('User has no external path set, cannot import asset');
-      return false;
-    }
-
-    if (!path.normalize(assetPath).match(new RegExp(`^${path.normalize(user.externalPath)}`))) {
-      this.logger.error("Asset must be within the user's external path");
-      return false;
-    }
-
     const existingAssetEntity = await this.assetRepository.getByLibraryIdAndOriginalPath(job.id, assetPath);
 
     let stats: Stats;
@@ -367,8 +356,6 @@ export class LibraryService {
       return false;
     }
 
-    const normalizedExternalPath = path.normalize(user.externalPath);
-
     this.logger.verbose(`Refreshing library: ${job.id}`);
     const crawledAssetPaths = (
       await this.storageRepository.crawl({
@@ -379,7 +366,7 @@ export class LibraryService {
       .map(path.normalize)
       .filter((assetPath) =>
         // Filter out paths that are not within the user's external path
-        assetPath.match(new RegExp(`^${normalizedExternalPath}`)),
+        assetPath.match(new RegExp(`^${user.externalPath}`)),
       );
 
     this.logger.debug(`Found ${crawledAssetPaths.length} assets when crawling import paths ${library.importPaths}`);