From 455a36b0fc1e83cacbaf352b05dde73a29de4dc4 Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Fri, 30 Jun 2023 12:25:08 -0400 Subject: [PATCH] fix(server): read file permission checks (#3046) --- server/src/domain/metadata/metadata.service.spec.ts | 4 ++-- server/src/domain/metadata/metadata.service.ts | 2 +- server/src/domain/user/user.core.ts | 2 +- server/src/immich/api-v1/asset/asset.service.ts | 13 ++++++------- .../src/infra/repositories/filesystem.provider.ts | 2 +- 5 files changed, 11 insertions(+), 12 deletions(-) diff --git a/server/src/domain/metadata/metadata.service.spec.ts b/server/src/domain/metadata/metadata.service.spec.ts index 1052d75850..1931e0b9c5 100644 --- a/server/src/domain/metadata/metadata.service.spec.ts +++ b/server/src/domain/metadata/metadata.service.spec.ts @@ -82,7 +82,7 @@ describe(MetadataService.name, () => { assetMock.save.mockResolvedValue(assetEntityStub.image); storageMock.checkFileExists.mockResolvedValue(true); await sut.handleSidecarDiscovery({ id: assetEntityStub.image.id }); - expect(storageMock.checkFileExists).toHaveBeenCalledWith('/original/path.ext.xmp', constants.W_OK); + expect(storageMock.checkFileExists).toHaveBeenCalledWith('/original/path.ext.xmp', constants.R_OK); expect(assetMock.save).toHaveBeenCalledWith({ id: assetEntityStub.image.id, sidecarPath: '/original/path.ext.xmp', @@ -94,7 +94,7 @@ describe(MetadataService.name, () => { assetMock.save.mockResolvedValue(assetEntityStub.video); storageMock.checkFileExists.mockResolvedValue(true); await sut.handleSidecarDiscovery({ id: assetEntityStub.video.id }); - expect(storageMock.checkFileExists).toHaveBeenCalledWith('/original/path.ext.xmp', constants.W_OK); + expect(storageMock.checkFileExists).toHaveBeenCalledWith('/original/path.ext.xmp', constants.R_OK); expect(assetMock.save).toHaveBeenCalledWith({ id: assetEntityStub.image.id, sidecarPath: '/original/path.ext.xmp', diff --git a/server/src/domain/metadata/metadata.service.ts b/server/src/domain/metadata/metadata.service.ts index e244ca6a1f..1570f51413 100644 --- a/server/src/domain/metadata/metadata.service.ts +++ b/server/src/domain/metadata/metadata.service.ts @@ -42,7 +42,7 @@ export class MetadataService { } const sidecarPath = `${asset.originalPath}.xmp`; - const exists = await this.storageRepository.checkFileExists(sidecarPath, constants.W_OK); + const exists = await this.storageRepository.checkFileExists(sidecarPath, constants.R_OK); if (!exists) { return false; } diff --git a/server/src/domain/user/user.core.ts b/server/src/domain/user/user.core.ts index 2b3ed40743..1b3e872256 100644 --- a/server/src/domain/user/user.core.ts +++ b/server/src/domain/user/user.core.ts @@ -112,7 +112,7 @@ export class UserCore { if (!user.profileImagePath) { throw new NotFoundException('User does not have a profile image'); } - await fs.access(user.profileImagePath, constants.R_OK | constants.W_OK); + await fs.access(user.profileImagePath, constants.R_OK); return createReadStream(user.profileImagePath); } diff --git a/server/src/immich/api-v1/asset/asset.service.ts b/server/src/immich/api-v1/asset/asset.service.ts index 1b1dd00ee7..9a275c66d1 100644 --- a/server/src/immich/api-v1/asset/asset.service.ts +++ b/server/src/immich/api-v1/asset/asset.service.ts @@ -24,9 +24,8 @@ import { StreamableFile, } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; -import { R_OK, W_OK } from 'constants'; import { Response as Res } from 'express'; -import { createReadStream, stat } from 'fs'; +import { constants, createReadStream, stat } from 'fs'; import fs from 'fs/promises'; import mime from 'mime-types'; import path from 'path'; @@ -156,7 +155,7 @@ export class AssetService { continue; } - const exists = await this.storageRepository.checkFileExists(filepath, R_OK); + const exists = await this.storageRepository.checkFileExists(filepath, constants.R_OK); if (!exists) { throw new BadRequestException('File does not exist'); } @@ -307,7 +306,7 @@ export class AssetService { let videoPath = asset.originalPath; let mimeType = asset.mimeType; - await fs.access(videoPath, R_OK | W_OK); + await fs.access(videoPath, constants.R_OK); if (asset.encodedVideoPath) { videoPath = asset.encodedVideoPath == '' ? String(asset.originalPath) : String(asset.encodedVideoPath); @@ -355,8 +354,8 @@ export class AssetService { } return this.streamFile(videoPath, res, headers, mimeType); - } catch (e) { - this.logger.error(`Error serving VIDEO asset=${asset.id}`); + } catch (e: Error | any) { + this.logger.error(`Error serving VIDEO asset=${asset.id}`, e?.stack); throw new InternalServerErrorException(`Failed to serve video asset ${e}`, 'ServeFile'); } } @@ -632,7 +631,7 @@ export class AssetService { return; } - await fs.access(filepath, R_OK); + await fs.access(filepath, constants.R_OK); return new StreamableFile(createReadStream(filepath)); } diff --git a/server/src/infra/repositories/filesystem.provider.ts b/server/src/infra/repositories/filesystem.provider.ts index d82e776c82..62388201bc 100644 --- a/server/src/infra/repositories/filesystem.provider.ts +++ b/server/src/infra/repositories/filesystem.provider.ts @@ -23,7 +23,7 @@ export class FilesystemProvider implements IStorageRepository { async createReadStream(filepath: string, mimeType?: string | null): Promise { const { size } = await fs.stat(filepath); - await fs.access(filepath, constants.R_OK | constants.W_OK); + await fs.access(filepath, constants.R_OK); return { stream: createReadStream(filepath), length: size,