fix(server): only allow absolute import paths (#13642)

fix: only allow absolute paths
immich-app · Oct 21, 2024 · b411e30 · b411e30
1 parent 56bebd0
commit b411e30
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 7 deletions.
diff --git a/e2e/src/api/specs/library.e2e-spec.ts b/e2e/src/api/specs/library.e2e-spec.ts
@@ -633,6 +633,29 @@ describe('/libraries', () => {
       });
     });
 
+    it("should fail if path isn't absolute", async () => {
+      const pathToTest = `relative/path`;
+
+      const cwd = process.cwd();
+      // Create directory in cwd
+      utils.createDirectory(`${cwd}/${pathToTest}`);
+
+      const response = await utils.validateLibrary(admin.accessToken, library.id, {
+        importPaths: [pathToTest],
+      });
+
+      utils.removeDirectory(`${cwd}/${pathToTest}`);
+
+      expect(response.importPaths?.length).toEqual(1);
+      const pathResponse = response?.importPaths?.at(0);
+
+      expect(pathResponse).toEqual({
+        importPath: pathToTest,
+        isValid: false,
+        message: expect.stringMatching('Import path must be absolute, try /usr/src/app/relative/path'),
+      });
+    });
+
     it('should fail if path is a file', async () => {
       const pathToTest = `${testAssetDirInternal}/albums/nature/el_torcal_rocks.jpg`;
 

diff --git a/server/src/services/library.service.spec.ts b/server/src/services/library.service.spec.ts
@@ -907,7 +907,9 @@ describe(LibraryService.name, () => {
       storageMock.stat.mockResolvedValue({ isDirectory: () => true } as Stats);
       storageMock.checkFileExists.mockResolvedValue(true);
 
-      await expect(sut.update('library-id', { importPaths: ['foo/bar'] })).resolves.toEqual(
+      const cwd = process.cwd();
+
+      await expect(sut.update('library-id', { importPaths: [`${cwd}/foo/bar`] })).resolves.toEqual(
         mapLibrary(libraryStub.externalLibrary1),
       );
       expect(libraryMock.update).toHaveBeenCalledWith(expect.objectContaining({ id: 'library-id' }));
@@ -1300,14 +1302,31 @@ describe(LibraryService.name, () => {
       });
     });
 
+    it('should detect when import path is not absolute', async () => {
+      const cwd = process.cwd();
+
+      await expect(sut.validate('library-id', { importPaths: ['relative/path'] })).resolves.toEqual({
+        importPaths: [
+          {
+            importPath: 'relative/path',
+            isValid: false,
+            message: `Import path must be absolute, try ${cwd}/relative/path`,
+          },
+        ],
+      });
+    });
+
     it('should detect when import path is in immich media folder', async () => {
       storageMock.stat.mockResolvedValue({ isDirectory: () => true } as Stats);
-      const validImport = libraryStub.hasImmichPaths.importPaths[1];
+      const cwd = process.cwd();
+
+      const validImport = `${cwd}/${libraryStub.hasImmichPaths.importPaths[1]}`;
       storageMock.checkFileExists.mockImplementation((importPath) => Promise.resolve(importPath === validImport));
 
-      await expect(
-        sut.validate('library-id', { importPaths: libraryStub.hasImmichPaths.importPaths }),
-      ).resolves.toEqual({
+      const pathStubs = libraryStub.hasImmichPaths.importPaths;
+      const importPaths = [pathStubs[0], validImport, pathStubs[2]];
+
+      await expect(sut.validate('library-id', { importPaths })).resolves.toEqual({
         importPaths: [
           {
             importPath: libraryStub.hasImmichPaths.importPaths[0],

diff --git a/server/src/services/library.service.ts b/server/src/services/library.service.ts
@@ -1,6 +1,6 @@
 import { BadRequestException, Injectable } from '@nestjs/common';
 import { R_OK } from 'node:constants';
-import path, { basename, parse } from 'node:path';
+import path, { basename, isAbsolute, parse } from 'node:path';
 import picomatch from 'picomatch';
 import { StorageCore } from 'src/cores/storage.core';
 import { OnEvent } from 'src/decorators';
@@ -268,6 +268,11 @@ export class LibraryService extends BaseService {
       return validation;
     }
 
+    if (!isAbsolute(importPath)) {
+      validation.message = `Import path must be absolute, try ${path.resolve(importPath)}`;
+      return validation;
+    }
+
     try {
       const stat = await this.storageRepository.stat(importPath);
       if (!stat.isDirectory()) {

diff --git a/server/test/fixtures/library.stub.ts b/server/test/fixtures/library.stub.ts
@@ -68,7 +68,7 @@ export const libraryStub = {
     assets: [],
     owner: userStub.admin,
     ownerId: 'user-id',
-    importPaths: ['upload/thumbs', '/xyz', 'upload/library'],
+    importPaths: ['upload/thumbs', 'xyz', 'upload/library'],
     createdAt: new Date('2023-01-01'),
     updatedAt: new Date('2023-01-01'),
     refreshedAt: null,