From 83b63ca12eabde7dfa4fbccda4490989441f1b93 Mon Sep 17 00:00:00 2001
From: Jonathan Jogenfors <jonathan@jogenfors.se>
Date: Tue, 10 Oct 2023 15:44:43 +0200
Subject: [PATCH] fix: only check external path once (#4419)

---
 .../domain/library/library.service.spec.ts    | 55 -------------------
 server/src/domain/library/library.service.ts  | 15 +----
 2 files changed, 1 insertion(+), 69 deletions(-)

diff --git a/server/src/domain/library/library.service.spec.ts b/server/src/domain/library/library.service.spec.ts
index 3f56c3100f..4591fb0b86 100644
--- a/server/src/domain/library/library.service.spec.ts
+++ b/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());
 
diff --git a/server/src/domain/library/library.service.ts b/server/src/domain/library/library.service.ts
index 44c60d4bc1..08a128d103 100644
--- a/server/src/domain/library/library.service.ts
+++ b/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}`);