feat(server,web) Semantic import path validation (#7076)

* add library validation api

* chore: open api

* show warning i UI

* add flex row

* fix e2e

* tests

* fix tests

* enforce path validation

* enforce validation on refresh

* return 400 on bad import path

* add limits to import paths

* set response code to 200

* fix e2e

* fix lint

* fix test

* restore e2e folder

* fix import

* use startsWith

* icon color

* notify user of failed validation

* add parent div to validation

* add docs to the import validation

* improve library troubleshooting docs

* fix button alignment

---------

Co-authored-by: Alex Tran <alex.tran1502@gmail.com>
This commit is contained in:
Jonathan Jogenfors 2024-02-20 16:53:12 +01:00 committed by GitHub
parent e7a875eadd
commit b3c7bebbd4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
32 changed files with 1472 additions and 75 deletions

View file

@ -1,6 +1,6 @@
import { LibraryEntity, LibraryType } from '@app/infra/entities';
import { ApiProperty } from '@nestjs/swagger';
import { ArrayUnique, IsBoolean, IsEnum, IsNotEmpty, IsOptional, IsString } from 'class-validator';
import { ArrayMaxSize, ArrayUnique, IsBoolean, IsEnum, IsNotEmpty, IsOptional, IsString } from 'class-validator';
import { ValidateUUID } from '../domain.util';
export class CreateLibraryDto {
@ -21,12 +21,14 @@ export class CreateLibraryDto {
@IsString({ each: true })
@IsNotEmpty({ each: true })
@ArrayUnique()
@ArrayMaxSize(128)
importPaths?: string[];
@IsOptional()
@IsString({ each: true })
@IsNotEmpty({ each: true })
@ArrayUnique()
@ArrayMaxSize(128)
exclusionPatterns?: string[];
@IsOptional()
@ -48,12 +50,14 @@ export class UpdateLibraryDto {
@IsString({ each: true })
@IsNotEmpty({ each: true })
@ArrayUnique()
@ArrayMaxSize(128)
importPaths?: string[];
@IsOptional()
@IsNotEmpty({ each: true })
@IsString({ each: true })
@ArrayUnique()
@ArrayMaxSize(128)
exclusionPatterns?: string[];
}
@ -63,6 +67,32 @@ export class CrawlOptionsDto {
exclusionPatterns?: string[];
}
export class ValidateLibraryDto {
@IsOptional()
@IsString({ each: true })
@IsNotEmpty({ each: true })
@ArrayUnique()
@ArrayMaxSize(128)
importPaths?: string[];
@IsOptional()
@IsNotEmpty({ each: true })
@IsString({ each: true })
@ArrayUnique()
@ArrayMaxSize(128)
exclusionPatterns?: string[];
}
export class ValidateLibraryResponseDto {
importPaths?: ValidateLibraryImportPathResponseDto[];
}
export class ValidateLibraryImportPathResponseDto {
importPath!: string;
isValid?: boolean = false;
message?: string;
}
export class LibrarySearchDto {
@ValidateUUID({ optional: true })
userId?: string;

View file

@ -54,12 +54,6 @@ describe(LibraryService.name, () => {
cryptoMock = newCryptoRepositoryMock();
storageMock = newStorageRepositoryMock();
storageMock.stat.mockResolvedValue({
size: 100,
mtime: new Date('2023-01-01'),
ctime: new Date('2023-01-01'),
} as Stats);
// Always validate owner access for library.
accessMock.library.checkOwnerAccess.mockImplementation(async (_, libraryIds) => libraryIds);
@ -270,6 +264,39 @@ describe(LibraryService.name, () => {
await expect(sut.handleQueueAssetRefresh(mockLibraryJob)).resolves.toBe(false);
});
it('should ignore import paths that do not exist', async () => {
storageMock.stat.mockImplementation((path): Promise<Stats> => {
if (path === libraryStub.externalLibraryWithImportPaths1.importPaths[0]) {
const error = { code: 'ENOENT' } as any;
throw error;
}
return Promise.resolve({
isDirectory: () => true,
} as Stats);
});
storageMock.checkFileExists.mockResolvedValue(true);
const mockLibraryJob: ILibraryRefreshJob = {
id: libraryStub.externalLibraryWithImportPaths1.id,
refreshModifiedFiles: false,
refreshAllFiles: false,
};
libraryMock.get.mockResolvedValue(libraryStub.externalLibraryWithImportPaths1);
storageMock.crawl.mockResolvedValue([]);
assetMock.getByLibraryId.mockResolvedValue([]);
libraryMock.getOnlineAssetPaths.mockResolvedValue([]);
userMock.get.mockResolvedValue(userStub.externalPathRoot);
await sut.handleQueueAssetRefresh(mockLibraryJob);
expect(storageMock.crawl).toHaveBeenCalledWith({
pathsToCrawl: [libraryStub.externalLibraryWithImportPaths1.importPaths[1]],
exclusionPatterns: [],
});
});
});
describe('handleAssetRefresh', () => {
@ -278,6 +305,12 @@ describe(LibraryService.name, () => {
beforeEach(() => {
mockUser = userStub.externalPath1;
userMock.get.mockResolvedValue(mockUser);
storageMock.stat.mockResolvedValue({
size: 100,
mtime: new Date('2023-01-01'),
ctime: new Date('2023-01-01'),
} as Stats);
});
it('should reject an unknown file extension', async () => {
@ -1104,13 +1137,19 @@ describe(LibraryService.name, () => {
libraryMock.update.mockResolvedValue(libraryStub.externalLibraryWithImportPaths1);
libraryMock.get.mockResolvedValue(libraryStub.externalLibraryWithImportPaths1);
await expect(sut.update(authStub.admin, authStub.admin.user.id, { importPaths: ['/foo'] })).resolves.toEqual(
mapLibrary(libraryStub.externalLibraryWithImportPaths1),
);
storageMock.stat.mockResolvedValue({
isDirectory: () => true,
} as Stats);
storageMock.checkFileExists.mockResolvedValue(true);
await expect(
sut.update(authStub.external1, authStub.external1.user.id, { importPaths: ['/data/user1/foo'] }),
).resolves.toEqual(mapLibrary(libraryStub.externalLibraryWithImportPaths1));
expect(libraryMock.update).toHaveBeenCalledWith(
expect.objectContaining({
id: authStub.admin.user.id,
id: authStub.external1.user.id,
}),
);
expect(storageMock.watch).toHaveBeenCalledWith(
@ -1142,7 +1181,7 @@ describe(LibraryService.name, () => {
});
});
describe('watchAll new', () => {
describe('watchAll', () => {
describe('watching disabled', () => {
beforeEach(async () => {
configMock.load.mockResolvedValue(systemConfigStub.libraryWatchDisabled);
@ -1523,4 +1562,121 @@ describe(LibraryService.name, () => {
]);
});
});
describe('validate', () => {
it('should validate directory', async () => {
storageMock.stat.mockResolvedValue({
isDirectory: () => true,
} as Stats);
storageMock.checkFileExists.mockResolvedValue(true);
const result = await sut.validate(authStub.external1, libraryStub.externalLibraryWithImportPaths1.id, {
importPaths: ['/data/user1/'],
});
expect(result.importPaths).toEqual([
{
importPath: '/data/user1/',
isValid: true,
message: undefined,
},
]);
});
it('should error when no external path is set', async () => {
await expect(
sut.validate(authStub.admin, libraryStub.externalLibrary1.id, { importPaths: ['/photos'] }),
).rejects.toBeInstanceOf(BadRequestException);
});
it('should detect when path is outside external path', async () => {
const result = await sut.validate(authStub.external1, libraryStub.externalLibraryWithImportPaths1.id, {
importPaths: ['/data/user2'],
});
expect(result.importPaths).toEqual([
{
importPath: '/data/user2',
isValid: false,
message: "Not contained in user's external path",
},
]);
});
it('should detect when path does not exist', async () => {
storageMock.stat.mockImplementation(() => {
const error = { code: 'ENOENT' } as any;
throw error;
});
const result = await sut.validate(authStub.external1, libraryStub.externalLibraryWithImportPaths1.id, {
importPaths: ['/data/user1/'],
});
expect(result.importPaths).toEqual([
{
importPath: '/data/user1/',
isValid: false,
message: 'Path does not exist (ENOENT)',
},
]);
});
it('should detect when path is not a directory', async () => {
storageMock.stat.mockResolvedValue({
isDirectory: () => false,
} as Stats);
const result = await sut.validate(authStub.external1, libraryStub.externalLibraryWithImportPaths1.id, {
importPaths: ['/data/user1/file'],
});
expect(result.importPaths).toEqual([
{
importPath: '/data/user1/file',
isValid: false,
message: 'Not a directory',
},
]);
});
it('should return an unknown exception from stat', async () => {
storageMock.stat.mockImplementation(() => {
throw new Error('Unknown error');
});
const result = await sut.validate(authStub.external1, libraryStub.externalLibraryWithImportPaths1.id, {
importPaths: ['/data/user1/'],
});
expect(result.importPaths).toEqual([
{
importPath: '/data/user1/',
isValid: false,
message: 'Error: Unknown error',
},
]);
});
it('should detect when access rights are missing', async () => {
storageMock.stat.mockResolvedValue({
isDirectory: () => true,
} as Stats);
storageMock.checkFileExists.mockResolvedValue(false);
const result = await sut.validate(authStub.external1, libraryStub.externalLibraryWithImportPaths1.id, {
importPaths: ['/data/user1/'],
});
expect(result.importPaths).toEqual([
{
importPath: '/data/user1/',
isValid: false,
message: 'Lacking read permission for folder',
},
]);
});
});
});

View file

@ -30,6 +30,9 @@ import {
LibraryStatsResponseDto,
ScanLibraryDto,
UpdateLibraryDto,
ValidateLibraryDto,
ValidateLibraryImportPathResponseDto,
ValidateLibraryResponseDto,
mapLibrary,
} from './library.dto';
@ -270,10 +273,81 @@ export class LibraryService extends EventEmitter {
);
}
private async validateImportPath(importPath: string): Promise<ValidateLibraryImportPathResponseDto> {
const validation = new ValidateLibraryImportPathResponseDto();
validation.importPath = importPath;
try {
const stat = await this.storageRepository.stat(importPath);
if (!stat.isDirectory()) {
validation.message = 'Not a directory';
return validation;
}
} catch (error: any) {
if (error.code === 'ENOENT') {
validation.message = 'Path does not exist (ENOENT)';
return validation;
}
validation.message = String(error);
return validation;
}
const access = await this.storageRepository.checkFileExists(importPath, R_OK);
if (!access) {
validation.message = 'Lacking read permission for folder';
return validation;
}
validation.isValid = true;
return validation;
}
public async validate(auth: AuthDto, id: string, dto: ValidateLibraryDto): Promise<ValidateLibraryResponseDto> {
await this.access.requirePermission(auth, Permission.LIBRARY_UPDATE, id);
if (!auth.user.externalPath) {
throw new BadRequestException('User has no external path set');
}
const response = new ValidateLibraryResponseDto();
if (dto.importPaths) {
response.importPaths = await Promise.all(
dto.importPaths.map(async (importPath) => {
const normalizedPath = path.normalize(importPath);
if (!this.isInExternalPath(normalizedPath, auth.user.externalPath)) {
const validation = new ValidateLibraryImportPathResponseDto();
validation.importPath = importPath;
validation.message = `Not contained in user's external path`;
return validation;
}
return await this.validateImportPath(importPath);
}),
);
}
return response;
}
async update(auth: AuthDto, id: string, dto: UpdateLibraryDto): Promise<LibraryResponseDto> {
await this.access.requirePermission(auth, Permission.LIBRARY_UPDATE, id);
const library = await this.repository.update({ id, ...dto });
if (dto.importPaths) {
const validation = await this.validate(auth, id, { importPaths: dto.importPaths });
if (validation.importPaths) {
for (const path of validation.importPaths) {
if (!path.isValid) {
throw new BadRequestException(`Invalid import path: ${path.message}`);
}
}
}
}
if (dto.importPaths || dto.exclusionPatterns) {
// Re-watch library to use new paths and/or exclusion patterns
await this.watch(id);
@ -509,6 +583,14 @@ export class LibraryService extends EventEmitter {
return true;
}
// Check if a given path is in a user's external path. Both arguments are assumed to be normalized
private isInExternalPath(filePath: string, externalPath: string | null): boolean {
if (externalPath === null) {
return false;
}
return filePath.startsWith(externalPath);
}
async handleQueueAssetRefresh(job: ILibraryRefreshJob): Promise<boolean> {
const library = await this.repository.get(job.id);
if (!library || library.type !== LibraryType.EXTERNAL) {
@ -523,17 +605,31 @@ export class LibraryService extends EventEmitter {
}
this.logger.verbose(`Refreshing library: ${job.id}`);
const pathValidation = await Promise.all(
library.importPaths.map(async (importPath) => await this.validateImportPath(importPath)),
);
const validImportPaths = pathValidation
.map((validation) => {
if (!validation.isValid) {
this.logger.error(`Skipping invalid import path: ${validation.importPath}. Reason: ${validation.message}`);
}
return validation;
})
.filter((validation) => validation.isValid)
.map((validation) => validation.importPath);
const rawPaths = await this.storageRepository.crawl({
pathsToCrawl: library.importPaths,
pathsToCrawl: validImportPaths,
exclusionPatterns: library.exclusionPatterns,
});
const crawledAssetPaths = rawPaths
// Normalize file paths. This is important to prevent security issues like path traversal
.map((filePath) => path.normalize(filePath))
.filter((assetPath) =>
// Filter out paths that are not within the user's external path
assetPath.match(new RegExp(`^${user.externalPath}`)),
) as string[];
// Filter out paths that are not within the user's external path
.filter((assetPath) => this.isInExternalPath(assetPath, user.externalPath)) as string[];
this.logger.debug(`Found ${crawledAssetPaths.length} asset(s) when crawling import paths ${library.importPaths}`);
const assetsInLibrary = await this.assetRepository.getByLibraryId([job.id]);

View file

@ -6,8 +6,10 @@ import {
LibraryResponseDto as ResponseDto,
ScanLibraryDto,
UpdateLibraryDto as UpdateDto,
ValidateLibraryDto,
ValidateLibraryResponseDto,
} from '@app/domain';
import { Body, Controller, Delete, Get, Param, Post, Put } from '@nestjs/common';
import { Body, Controller, Delete, Get, HttpCode, Param, Post, Put } from '@nestjs/common';
import { ApiTags } from '@nestjs/swagger';
import { Auth, Authenticated } from '../app.guard';
import { UseValidation } from '../app.utils';
@ -40,6 +42,16 @@ export class LibraryController {
return this.service.get(auth, id);
}
@Post(':id/validate')
@HttpCode(200)
validate(
@Auth() auth: AuthDto,
@Param() { id }: UUIDParamDto,
@Body() dto: ValidateLibraryDto,
): Promise<ValidateLibraryResponseDto> {
return this.service.validate(auth, id, dto);
}
@Delete(':id')
deleteLibrary(@Auth() auth: AuthDto, @Param() { id }: UUIDParamDto): Promise<void> {
return this.service.delete(auth, id);

View file

@ -164,7 +164,21 @@ export function searchAssetBuilder(
builder.andWhere(_.omitBy(path, _.isUndefined));
const status = _.pick(options, ['isExternal', 'isFavorite', 'isOffline', 'isReadOnly', 'isVisible', 'type']);
const { isArchived, isEncoded, isMotion, withArchived } = options;
const {
isArchived,
isEncoded,
isMotion,
withArchived,
isNotInAlbum,
withFaces,
withPeople,
withSmartInfo,
personIds,
withExif,
withStacked,
trashedAfter,
trashedBefore,
} = options;
builder.andWhere(
_.omitBy(
{
@ -177,38 +191,38 @@ export function searchAssetBuilder(
),
);
if (options.isNotInAlbum) {
if (isNotInAlbum) {
builder
.leftJoin(`${builder.alias}.albums`, 'albums')
.andWhere('albums.id IS NULL')
.andWhere(`${builder.alias}.isVisible = true`);
}
if (options.withFaces || options.withPeople) {
if (withFaces || withPeople) {
builder.leftJoinAndSelect(`${builder.alias}.faces`, 'faces');
}
if (options.withPeople) {
if (withPeople) {
builder.leftJoinAndSelect(`${builder.alias}.person`, 'person');
}
if (options.withSmartInfo) {
if (withSmartInfo) {
builder.leftJoinAndSelect(`${builder.alias}.smartInfo`, 'smartInfo');
}
if (options.personIds && options.personIds.length > 0) {
if (personIds && personIds.length > 0) {
builder
.leftJoin(`${builder.alias}.faces`, 'faces')
.andWhere('faces.personId IN (:...personIds)', { personIds: options.personIds })
.andWhere('faces.personId IN (:...personIds)', { personIds: personIds })
.addGroupBy(`${builder.alias}.id`)
.having('COUNT(faces.id) = :personCount', { personCount: options.personIds.length });
.having('COUNT(faces.id) = :personCount', { personCount: personIds.length });
if (options.withExif) {
if (withExif) {
builder.addGroupBy('exifInfo.assetId');
}
}
if (options.withStacked) {
if (withStacked) {
builder
.leftJoinAndSelect(`${builder.alias}.stack`, 'stack')
.leftJoinAndSelect('stack.assets', 'stackedAssets')
@ -217,8 +231,7 @@ export function searchAssetBuilder(
);
}
const withDeleted =
options.withDeleted ?? (options.trashedAfter !== undefined || options.trashedBefore !== undefined);
const withDeleted = options.withDeleted ?? (trashedAfter !== undefined || trashedBefore !== undefined);
if (withDeleted) {
builder.withDeleted();
}

View file

@ -12,12 +12,24 @@ import archiver from 'archiver';
import chokidar, { WatchOptions } from 'chokidar';
import { glob } from 'glob';
import { constants, createReadStream, existsSync, mkdirSync } from 'node:fs';
import fs, { copyFile, readdir, rename, utimes, writeFile } from 'node:fs/promises';
import fs, { copyFile, readdir, rename, stat, utimes, writeFile } from 'node:fs/promises';
import path from 'node:path';
export class FilesystemProvider implements IStorageRepository {
private logger = new ImmichLogger(FilesystemProvider.name);
readdir = readdir;
copyFile = copyFile;
stat = stat;
writeFile = writeFile;
rename = rename;
utimes = utimes;
createZipStream(): ImmichZipStream {
const archive = archiver('zip', { store: true });
@ -50,14 +62,6 @@ export class FilesystemProvider implements IStorageRepository {
}
}
writeFile = writeFile;
rename = rename;
copyFile = copyFile;
utimes = utimes;
async checkFileExists(filepath: string, mode = constants.F_OK): Promise<boolean> {
try {
await fs.access(filepath, mode);
@ -79,8 +83,6 @@ export class FilesystemProvider implements IStorageRepository {
}
}
stat = fs.stat;
async unlinkDir(folder: string, options: { recursive?: boolean; force?: boolean }) {
await fs.rm(folder, options);
}
@ -146,6 +148,4 @@ export class FilesystemProvider implements IStorageRepository {
return () => watcher.close();
}
readdir = readdir;
}