From 39d9be30f9f443de597a9f5d6fe59f2d1d9609ba Mon Sep 17 00:00:00 2001
From: neon <18559658+neonsolstice@users.noreply.github.com>
Date: Tue, 7 Jul 2026 17:02:46 +0000
Subject: [PATCH] feat(library): support LIBRARY_BROWSE_ROOT to restrict file
path access
Introduce a LIBRARY_BROWSE_ROOT environment variable (defaulting to the
current directory) to restrict directories users can browse and select when
setting up library paths. Add a PathPolicyService on the backend to enforce
path containment checks securely, and update the frontend FolderPickerModal
to prevent navigating above the configured browse root.
Closes #567
---
.env.example | 4 +
README.md | 2 +
.../library/components/FolderPickerModal.vue | 64 ++++++++++---
.../__tests__/FolderPickerModal.spec.ts | 91 +++++++++++++++++++
docker-compose.yml | 1 +
docs/DEVELOPMENT.md | 19 ++--
packages/types/src/path.ts | 4 +
server/.env.example | 4 +
server/src/config/config.test.ts | 15 +++
server/src/config/config.ts | 2 +
server/src/config/env.validation.test.ts | 18 ++++
server/src/config/env.validation.ts | 4 +
server/src/modules/library/library.module.ts | 11 ++-
.../modules/library/library.service.test.ts | 43 ++++++++-
server/src/modules/library/library.service.ts | 32 +++++--
.../modules/path/path-policy.service.test.ts | 67 ++++++++++++++
.../src/modules/path/path-policy.service.ts | 78 ++++++++++++++++
.../src/modules/path/path.controller.test.ts | 11 ++-
server/src/modules/path/path.controller.ts | 11 ++-
server/src/modules/path/path.module.test.ts | 4 +-
server/src/modules/path/path.module.ts | 4 +-
server/src/modules/path/path.service.test.ts | 34 ++++++-
server/src/modules/path/path.service.ts | 19 +++-
.../route-inventory/manifest.json | 4 +-
.../route-inventory/routes-05.json | 12 +++
25 files changed, 515 insertions(+), 43 deletions(-)
create mode 100644 client/src/features/library/components/__tests__/FolderPickerModal.spec.ts
create mode 100644 server/src/modules/path/path-policy.service.test.ts
create mode 100644 server/src/modules/path/path-policy.service.ts
diff --git a/.env.example b/.env.example
index 76a483d8f..c1cd00adb 100644
--- a/.env.example
+++ b/.env.example
@@ -40,6 +40,10 @@ NODE_MAX_OLD_SPACE_SIZE=2048
# CORS origin for the frontend when served from a different domain than APP_URL
# CLIENT_URL=https://app.example.com
+# Container folder used as the top of the library creation folder picker.
+# Defaults to / when unset. Set to /books to hide other container root folders.
+# LIBRARY_BROWSE_ROOT=/books
+
# Set to false only if your platform manages bind mount ownership externally.
# BOOKORBIT_FIX_PERMISSIONS=true
diff --git a/README.md b/README.md
index e42dd87c7..976f31774 100644
--- a/README.md
+++ b/README.md
@@ -94,6 +94,8 @@ JWT_SECRET= # signs login tokens - openssl rand -hex 32
SETUP_BOOTSTRAP_TOKEN= # one-time setup wizard token - openssl rand -hex 16
```
+Optionally set `LIBRARY_BROWSE_ROOT=/books` to start the library folder picker at `/books` instead of `/`.
+
Then start:
```bash
diff --git a/client/src/features/library/components/FolderPickerModal.vue b/client/src/features/library/components/FolderPickerModal.vue
index ca9cb2b7d..0f7aac862 100644
--- a/client/src/features/library/components/FolderPickerModal.vue
+++ b/client/src/features/library/components/FolderPickerModal.vue
@@ -1,7 +1,7 @@
diff --git a/client/src/features/library/components/__tests__/FolderPickerModal.spec.ts b/client/src/features/library/components/__tests__/FolderPickerModal.spec.ts
new file mode 100644
index 000000000..0f12897e1
--- /dev/null
+++ b/client/src/features/library/components/__tests__/FolderPickerModal.spec.ts
@@ -0,0 +1,91 @@
+import { mount, flushPromises } from '@vue/test-utils'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import FolderPickerModal from '../FolderPickerModal.vue'
+
+const apiMock = vi.hoisted(() => vi.fn<(input: RequestInfo | URL, init?: RequestInit) => Promise>())
+
+vi.mock('@/lib/api', () => ({
+ api: apiMock,
+}))
+
+const tooltipStubs = {
+ Teleport: true,
+ Tooltip: { template: '
' },
+ TooltipTrigger: { template: '
' },
+ TooltipContent: { template: '
' },
+}
+
+describe('FolderPickerModal', () => {
+ beforeEach(() => {
+ apiMock.mockReset()
+ apiMock.mockImplementation(async (input, init) => {
+ const url = String(input)
+ if (url === '/api/v1/path/config') return jsonResponse({ root: '/books' })
+ if (url === '/api/v1/path?path=%2Fbooks') return jsonResponse([{ name: 'Sci-Fi', path: '/books/Sci-Fi' }])
+ if (url === '/api/v1/path?path=%2Fbooks%2FSci-Fi') return jsonResponse([])
+ if (url === '/api/v1/path' && init?.method === 'POST') return jsonResponse({ name: 'New', path: '/books/New' })
+ return jsonResponse({}, 404)
+ })
+ })
+
+ it('loads the configured browse root before listing folders', async () => {
+ const wrapper = mountPicker()
+ await flushPromises()
+
+ expect(apiMock).toHaveBeenNthCalledWith(1, '/api/v1/path/config')
+ expect(apiMock).toHaveBeenNthCalledWith(2, '/api/v1/path?path=%2Fbooks')
+ expect(wrapper.text()).toContain('/books')
+ expect(wrapper.text()).toContain('Sci-Fi')
+ })
+
+ it('does not navigate above the configured browse root', async () => {
+ const wrapper = mountPicker()
+ await flushPromises()
+
+ await folderButton(wrapper, 'Sci-Fi').trigger('click')
+ await flushPromises()
+ await (wrapper.vm as unknown as { goUp: () => Promise }).goUp()
+
+ expect((wrapper.vm as unknown as { currentPath: string }).currentPath).toBe('/books')
+ })
+
+ it('creates folders under the configured browse root', async () => {
+ const wrapper = mountPicker()
+ await flushPromises()
+
+ await buttonByText(wrapper, 'New folder').trigger('click')
+ await wrapper.get('input[placeholder="New folder name"]').setValue('New')
+ await buttonByText(wrapper, 'Create').trigger('click')
+ await flushPromises()
+
+ const createCall = apiMock.mock.calls.find(([url, init]) => String(url) === '/api/v1/path' && init?.method === 'POST')
+ expect(JSON.parse(createCall?.[1]?.body as string)).toEqual({ parentPath: '/books', name: 'New' })
+ })
+})
+
+function mountPicker() {
+ return mount(FolderPickerModal, {
+ global: {
+ stubs: tooltipStubs,
+ },
+ })
+}
+
+function buttonByText(wrapper: ReturnType, text: string) {
+ const button = wrapper.findAll('button').find((candidate) => candidate.text().trim() === text)
+ if (!button) throw new Error(`Button not found: ${text}`)
+ return button
+}
+
+function folderButton(wrapper: ReturnType, text: string) {
+ const button = wrapper.findAll('button').find((candidate) => candidate.text().includes(text))
+ if (!button) throw new Error(`Folder button not found: ${text}`)
+ return button
+}
+
+function jsonResponse(body: unknown, status = 200): Response {
+ return new Response(JSON.stringify(body), {
+ status,
+ headers: { 'Content-Type': 'application/json' },
+ })
+}
diff --git a/docker-compose.yml b/docker-compose.yml
index 06a68631f..8892f9472 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -27,6 +27,7 @@ services:
PGID: ${PGID:-1000}
NODE_MAX_OLD_SPACE_SIZE: ${NODE_MAX_OLD_SPACE_SIZE:-auto}
BOOK_DOCK_PATH: ${BOOK_DOCK_PATH:-}
+ LIBRARY_BROWSE_ROOT: ${LIBRARY_BROWSE_ROOT:-}
depends_on:
postgres:
condition: service_healthy
diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md
index 3d1cd04be..225329a93 100644
--- a/docs/DEVELOPMENT.md
+++ b/docs/DEVELOPMENT.md
@@ -224,15 +224,16 @@ pnpm --filter @bookorbit/types dev
These are the variables you are most likely to need during development. See `server/.env.example` for the full list with comments.
-| Variable | Purpose | Default (dev) |
-| ----------------- | ---------------------------------- | --------------------------------------------------------- |
-| `DATABASE_URL` | PostgreSQL connection string | `postgres://bookorbit:bookorbit@localhost:5432/bookorbit` |
-| `PORT` | Server listen port | `3000` |
-| `NODE_ENV` | Runtime mode | `development` |
-| `JWT_SECRET` | Signing key for auth tokens | `change-me-in-production` |
-| `APP_DATA_PATH` | Storage for covers, avatars, cache | `../local/data` |
-| `APP_URL` | Base URL for email links | `http://localhost:5173` |
-| `SWAGGER_ENABLED` | Serve Swagger UI and OpenAPI JSON | `false` |
+| Variable | Purpose | Default (dev) |
+| --------------------- | ---------------------------------- | --------------------------------------------------------- |
+| `DATABASE_URL` | PostgreSQL connection string | `postgres://bookorbit:bookorbit@localhost:5432/bookorbit` |
+| `PORT` | Server listen port | `3000` |
+| `NODE_ENV` | Runtime mode | `development` |
+| `JWT_SECRET` | Signing key for auth tokens | `change-me-in-production` |
+| `APP_DATA_PATH` | Storage for covers, avatars, cache | `../local/data` |
+| `LIBRARY_BROWSE_ROOT` | Library folder picker root | `/` |
+| `APP_URL` | Base URL for email links | `http://localhost:5173` |
+| `SWAGGER_ENABLED` | Serve Swagger UI and OpenAPI JSON | `false` |
---
diff --git a/packages/types/src/path.ts b/packages/types/src/path.ts
index 2d3231cee..733fa0e41 100644
--- a/packages/types/src/path.ts
+++ b/packages/types/src/path.ts
@@ -3,6 +3,10 @@ export interface DirectoryEntry {
path: string;
}
+export interface PathConfig {
+ root: string;
+}
+
export interface CreateFolderRequest {
parentPath: string;
name: string;
diff --git a/server/.env.example b/server/.env.example
index d01465b0b..66e93b748 100644
--- a/server/.env.example
+++ b/server/.env.example
@@ -22,6 +22,10 @@ SETUP_BOOTSTRAP_TOKEN=
# ../local/data points to /local/data.
APP_DATA_PATH=../local/data
+# Top of the library creation folder picker. Defaults to / when unset.
+# Set this to the server-visible folder where users should create libraries.
+# LIBRARY_BROWSE_ROOT=../local/books
+
# File write scheduler and concurrency tuning (optional)
# FILE_WRITE_DEBOUNCE_MS=3000
# FILE_WRITE_MAX_CONCURRENT_WRITES=2
diff --git a/server/src/config/config.test.ts b/server/src/config/config.test.ts
index c90c73135..72a452b03 100644
--- a/server/src/config/config.test.ts
+++ b/server/src/config/config.test.ts
@@ -20,6 +20,7 @@ function resetEnv(): void {
delete process.env.SETUP_BOOTSTRAP_TOKEN;
delete process.env.APP_DATA_PATH;
delete process.env.BOOK_DOCK_PATH;
+ delete process.env.LIBRARY_BROWSE_ROOT;
delete process.env.FILE_WRITE_DEBOUNCE_MS;
delete process.env.FILE_WRITE_MAX_CONCURRENT_WRITES;
delete process.env.EMAIL_ENCRYPTION_KEY;
@@ -100,11 +101,13 @@ describe('config', () => {
process.env.APP_DATA_PATH = './tmp/bookorbit-data';
expect(storageConfig().appDataPath).toBe(resolve('./tmp/bookorbit-data'));
expect(storageConfig().bookDockPath).toBe(resolve('./tmp/bookorbit-data/book-dock'));
+ expect(storageConfig().libraryBrowseRoot).toBe(resolve('/'));
});
it('falls back storage path to /data when APP_DATA_PATH is not set', () => {
expect(storageConfig().appDataPath).toBe(resolve('/data'));
expect(storageConfig().bookDockPath).toBe(resolve('/data/book-dock'));
+ expect(storageConfig().libraryBrowseRoot).toBe(resolve('/'));
});
it('uses BOOK_DOCK_PATH as the Book Dock storage path when provided', () => {
@@ -114,6 +117,18 @@ describe('config', () => {
expect(storageConfig().bookDockPath).toBe(resolve('/books/bookdrop'));
});
+ it('uses LIBRARY_BROWSE_ROOT as the library folder picker root when provided', () => {
+ process.env.LIBRARY_BROWSE_ROOT = '/books';
+
+ expect(storageConfig().libraryBrowseRoot).toBe(resolve('/books'));
+ });
+
+ it('ignores blank LIBRARY_BROWSE_ROOT values from compose defaults', () => {
+ process.env.LIBRARY_BROWSE_ROOT = '';
+
+ expect(storageConfig().libraryBrowseRoot).toBe(resolve('/'));
+ });
+
it('ignores blank BOOK_DOCK_PATH values from compose defaults', () => {
process.env.APP_DATA_PATH = '/custom/data';
process.env.BOOK_DOCK_PATH = '';
diff --git a/server/src/config/config.ts b/server/src/config/config.ts
index 4ad5bc6c4..c78a89a77 100644
--- a/server/src/config/config.ts
+++ b/server/src/config/config.ts
@@ -28,10 +28,12 @@ export const authConfig = registerAs('auth', () => ({
export const storageConfig = registerAs('storage', () => {
const appDataPath = resolve(process.env.APP_DATA_PATH ?? '/data');
const bookDockPath = process.env.BOOK_DOCK_PATH?.trim();
+ const libraryBrowseRoot = process.env.LIBRARY_BROWSE_ROOT?.trim();
return {
appDataPath,
bookDockPath: resolve(bookDockPath || join(appDataPath, 'book-dock')),
+ libraryBrowseRoot: resolve(libraryBrowseRoot || '/'),
};
});
diff --git a/server/src/config/env.validation.test.ts b/server/src/config/env.validation.test.ts
index b7acdbb4a..bd864d78c 100644
--- a/server/src/config/env.validation.test.ts
+++ b/server/src/config/env.validation.test.ts
@@ -116,4 +116,22 @@ describe('validateEnv', () => {
}),
).not.toThrow();
});
+
+ it('accepts a custom library browse root path', () => {
+ expect(() =>
+ validateEnv({
+ ...BASE_ENV,
+ LIBRARY_BROWSE_ROOT: '/books',
+ }),
+ ).not.toThrow();
+ });
+
+ it('accepts an empty library browse root as an unset override', () => {
+ expect(() =>
+ validateEnv({
+ ...BASE_ENV,
+ LIBRARY_BROWSE_ROOT: '',
+ }),
+ ).not.toThrow();
+ });
});
diff --git a/server/src/config/env.validation.ts b/server/src/config/env.validation.ts
index b604e49b8..cee66a304 100644
--- a/server/src/config/env.validation.ts
+++ b/server/src/config/env.validation.ts
@@ -49,6 +49,10 @@ const envSchema = z.object({
.string()
.transform((val) => val.trim())
.optional(),
+ LIBRARY_BROWSE_ROOT: z
+ .string()
+ .transform((val) => val.trim())
+ .optional(),
FILE_WRITE_DEBOUNCE_MS: z.coerce.number().int().positive().optional(),
FILE_WRITE_MAX_CONCURRENT_WRITES: z.coerce.number().int().positive().optional(),
CLIENT_URL: z.string().url().optional(),
diff --git a/server/src/modules/library/library.module.ts b/server/src/modules/library/library.module.ts
index 1112451a5..b1f590781 100644
--- a/server/src/modules/library/library.module.ts
+++ b/server/src/modules/library/library.module.ts
@@ -5,6 +5,7 @@ import { AppSettingsModule } from '../app-settings/app-settings.module';
import { BookModule } from '../book/book.module';
import { FileWriteModule } from '../file-write/file-write.module';
import { NotificationModule } from '../notification/notification.module';
+import { PathModule } from '../path/path.module';
import { ScannerModule } from '../scanner/scanner.module';
import { BulkRenameService } from './bulk-rename.service';
import { LibraryController } from './library.controller';
@@ -12,7 +13,15 @@ import { LibraryRepository } from './library.repository';
import { LibraryService } from './library.service';
@Module({
- imports: [ScannerModule, AchievementModule, forwardRef(() => BookModule), FileWriteModule, forwardRef(() => NotificationModule), AppSettingsModule],
+ imports: [
+ ScannerModule,
+ AchievementModule,
+ forwardRef(() => BookModule),
+ FileWriteModule,
+ forwardRef(() => NotificationModule),
+ AppSettingsModule,
+ PathModule,
+ ],
controllers: [LibraryController],
providers: [LibraryService, LibraryRepository, BulkRenameService],
exports: [LibraryService, LibraryRepository],
diff --git a/server/src/modules/library/library.service.test.ts b/server/src/modules/library/library.service.test.ts
index 3ffb2d9a0..b54ca4baa 100644
--- a/server/src/modules/library/library.service.test.ts
+++ b/server/src/modules/library/library.service.test.ts
@@ -8,7 +8,7 @@ vi.mock('../scanner/lib/classify', () => ({
isPrimaryFormat: vi.fn(),
}));
-import { BadRequestException, ConflictException, InternalServerErrorException, NotFoundException } from '@nestjs/common';
+import { BadRequestException, ConflictException, ForbiddenException, InternalServerErrorException, NotFoundException } from '@nestjs/common';
import { EMPTY_CONTENT_FILTER_RULES } from '@bookorbit/types';
import { readdir, rm, stat } from 'fs/promises';
@@ -66,6 +66,10 @@ describe('LibraryService', () => {
const achievementEvents = {
emit: vi.fn(),
};
+ const pathPolicy = {
+ assertWithinBrowseRoot: vi.fn((path: string) => Promise.resolve(path)),
+ resolveBrowsePath: vi.fn((path: string) => Promise.resolve(path)),
+ };
let service: LibraryService;
@@ -79,12 +83,15 @@ describe('LibraryService', () => {
fileWatcherService as any,
fileWriteService as any,
achievementEvents as any,
+ pathPolicy as any,
);
mockStat.mockResolvedValue({ isDirectory: () => true } as Awaited>);
mockReaddir.mockResolvedValue([] as unknown as Awaited>);
mockRm.mockResolvedValue(undefined);
mockIsPrimaryFormat.mockReturnValue(false);
+ pathPolicy.assertWithinBrowseRoot.mockImplementation((path: string) => Promise.resolve(path));
+ pathPolicy.resolveBrowsePath.mockImplementation((path: string) => Promise.resolve(path));
});
it('findAll uses scoped folder query for non-superusers', async () => {
@@ -220,6 +227,16 @@ describe('LibraryService', () => {
await expect(service.create({ name: 'Dup', icon: 'BookOpen', folders: ['/x'] } as any)).rejects.toBeInstanceOf(ConflictException);
});
+ it('create rejects folders outside the configured browse root before inserting the library', async () => {
+ libraryRepo.findByName.mockResolvedValue([]);
+ pathPolicy.assertWithinBrowseRoot.mockRejectedValue(new ForbiddenException('outside root'));
+
+ await expect(service.create({ name: 'Sci-Fi', icon: 'BookOpen', folders: ['/outside'] } as any)).rejects.toBeInstanceOf(ForbiddenException);
+
+ expect(libraryRepo.insert).not.toHaveBeenCalled();
+ expect(libraryRepo.insertFolder).not.toHaveBeenCalled();
+ });
+
it('create rejects missing icons', async () => {
libraryRepo.findByName.mockResolvedValue([]);
@@ -248,6 +265,17 @@ describe('LibraryService', () => {
expect(fileWatcherService.stopWatcher).not.toHaveBeenCalled();
});
+ it('update rejects folders outside the configured browse root before changing the library', async () => {
+ libraryRepo.findById.mockResolvedValue([{ id: 3, name: 'Current', icon: 'BookOpen', watch: false }]);
+ pathPolicy.assertWithinBrowseRoot.mockRejectedValue(new ForbiddenException('outside root'));
+
+ await expect(service.update(3, { folders: ['/outside'] } as any)).rejects.toBeInstanceOf(ForbiddenException);
+
+ expect(libraryRepo.update).not.toHaveBeenCalled();
+ expect(libraryRepo.insertFolder).not.toHaveBeenCalled();
+ expect(libraryRepo.deleteFolder).not.toHaveBeenCalled();
+ });
+
it('update starts watcher when watch toggles on', async () => {
libraryRepo.findById.mockResolvedValue([{ id: 7, name: 'Current', icon: 'BookOpen', watch: false }]);
libraryRepo.update.mockResolvedValue([{ id: 7, name: 'Current', watch: true }]);
@@ -420,6 +448,19 @@ describe('LibraryService', () => {
expect(result.paths[1]).toEqual(expect.objectContaining({ overlapLibrary: 'Existing Library' }));
});
+ it('prescan reports paths outside the configured browse root without touching the filesystem', async () => {
+ libraryRepo.findAllFolderPaths.mockResolvedValue([]);
+ pathPolicy.resolveBrowsePath.mockRejectedValue(new ForbiddenException('outside root'));
+
+ const result = await service.prescan({ paths: ['/outside'] } as any);
+
+ expect(result).toEqual({
+ paths: [{ path: '/outside', accessible: false, fileCount: 0, error: 'Path is outside the configured library browse root' }],
+ totalFiles: 0,
+ });
+ expect(mockStat).not.toHaveBeenCalled();
+ });
+
it('prescan reports non-directory paths with explicit error', async () => {
libraryRepo.findAllFolderPaths.mockResolvedValue([]);
mockStat.mockResolvedValue({ isDirectory: () => false } as Awaited>);
diff --git a/server/src/modules/library/library.service.ts b/server/src/modules/library/library.service.ts
index 1298931c7..9f2fee09f 100644
--- a/server/src/modules/library/library.service.ts
+++ b/server/src/modules/library/library.service.ts
@@ -18,6 +18,7 @@ import { normalizeIconValue } from '../../common/utils/icon-value.utils';
import type { RequestUser } from '../../common/types/request-user';
import { AchievementEventsService, ACHIEVEMENT_EVENT_LIBRARY_CATALOG_CHANGED } from '../achievement/achievement-events.service';
import { FileWriteService } from '../file-write/file-write.service';
+import { PathPolicyService } from '../path/path-policy.service';
import { isPrimaryFormat } from '../scanner/lib/classify';
import { FileWatcherService } from '../scanner/file-watcher.service';
import { ScannerService } from '../scanner/scanner.service';
@@ -54,6 +55,7 @@ export class LibraryService {
private readonly fileWatcherService: FileWatcherService,
private readonly fileWriteService: FileWriteService,
private readonly achievementEvents: AchievementEventsService,
+ private readonly pathPolicy: PathPolicyService,
) {
this.appDataPath = this.config.get('storage.appDataPath')!;
}
@@ -102,6 +104,7 @@ export class LibraryService {
async create(dto: CreateLibraryDto) {
await this.assertNameAvailable(dto.name);
+ const folderPaths = await this.assertFolderPathsWithinBrowseRoot(dto.folders);
const icon = normalizeIconValue(dto.icon);
if (!icon) {
throw new BadRequestException('Icon is required');
@@ -135,7 +138,7 @@ export class LibraryService {
fileRenameEnabled: dto.fileRenameEnabled ?? false,
});
- const folders = await Promise.all(dto.folders.map((path) => this.libraryRepo.insertFolder({ libraryId: library.id, path })));
+ const folders = await Promise.all(folderPaths.map((path) => this.libraryRepo.insertFolder({ libraryId: library.id, path })));
if (library.watch) {
await this.fileWatcherService.startWatcher(
@@ -164,7 +167,8 @@ export class LibraryService {
await this.assertNameAvailable(dto.name, id);
}
- const { folders: folderPaths, ...fields } = dto;
+ const { folders: rawFolderPaths, ...fields } = dto;
+ const folderPaths = rawFolderPaths === undefined ? undefined : await this.assertFolderPathsWithinBrowseRoot(rawFolderPaths);
const icon = fields.icon !== undefined ? normalizeIconValue(fields.icon) : normalizeIconValue(existing.icon);
if (!icon) {
throw new BadRequestException('Icon is required');
@@ -235,9 +239,19 @@ export class LibraryService {
let error: string | undefined;
let overlapLibrary: string | undefined;
+ const resolvedInputPath = await this.pathPolicy.resolveBrowsePath(inputPath).catch(() => null);
+ if (resolvedInputPath === null) {
+ return {
+ path: inputPath,
+ accessible: false,
+ fileCount: 0,
+ error: 'Path is outside the configured library browse root',
+ };
+ }
+
let inputStat: Awaited> | null = null;
try {
- inputStat = await stat(inputPath);
+ inputStat = await stat(resolvedInputPath);
} catch (err) {
error = formatPrescanError(err);
}
@@ -245,11 +259,11 @@ export class LibraryService {
if (inputStat === null) {
accessible = false;
} else if (!inputStat.isDirectory()) {
- return { path: inputPath, accessible: false, fileCount: 0, error: 'Not a directory' };
+ return { path: resolvedInputPath, accessible: false, fileCount: 0, error: 'Not a directory' };
} else {
accessible = true;
try {
- fileCount = await countPrimaryFiles(inputPath);
+ fileCount = await countPrimaryFiles(resolvedInputPath);
} catch (err) {
accessible = false;
fileCount = 0;
@@ -258,13 +272,13 @@ export class LibraryService {
}
for (const existing of allFolderPaths) {
- if (pathsOverlap(inputPath, existing.path)) {
+ if (pathsOverlap(resolvedInputPath, existing.path)) {
overlapLibrary = existing.libraryName;
break;
}
}
- return { path: inputPath, accessible, fileCount, overlapLibrary, error };
+ return { path: resolvedInputPath, accessible, fileCount, overlapLibrary, error };
}),
);
@@ -361,6 +375,10 @@ export class LibraryService {
if (existing.length > 0) throw new ConflictException('A library with this name already exists');
}
+ private async assertFolderPathsWithinBrowseRoot(paths: string[]): Promise {
+ return Promise.all(paths.map((path) => this.pathPolicy.assertWithinBrowseRoot(path)));
+ }
+
private async cleanupCoverDirectories(bookIds: number[]): Promise {
const event = 'library.remove_cover_dirs';
const startedAt = Date.now();
diff --git a/server/src/modules/path/path-policy.service.test.ts b/server/src/modules/path/path-policy.service.test.ts
new file mode 100644
index 000000000..c70148d6d
--- /dev/null
+++ b/server/src/modules/path/path-policy.service.test.ts
@@ -0,0 +1,67 @@
+import { ForbiddenException } from '@nestjs/common';
+import { mkdtemp, mkdir, rm, symlink } from 'fs/promises';
+import { tmpdir } from 'os';
+import { join, resolve } from 'path';
+
+import { PathPolicyService } from './path-policy.service';
+
+describe('PathPolicyService', () => {
+ const tempRoots: string[] = [];
+
+ afterEach(async () => {
+ await Promise.all(tempRoots.splice(0).map((path) => rm(path, { recursive: true, force: true })));
+ });
+
+ it('uses the configured browse root when no path is provided', async () => {
+ const root = await makeTempRoot();
+ const service = makeService(root);
+
+ await expect(service.resolveBrowsePath()).resolves.toBe(root);
+ expect(service.getBrowseRoot()).toBe(root);
+ });
+
+ it('allows paths equal to or below the configured browse root', async () => {
+ const root = await makeTempRoot();
+ const child = join(root, 'authors');
+ await mkdir(child);
+ const service = makeService(root);
+
+ await expect(service.assertWithinBrowseRoot(root)).resolves.toBe(root);
+ await expect(service.assertWithinBrowseRoot(child)).resolves.toBe(child);
+ await expect(service.isWithinBrowseRoot(child)).resolves.toBe(true);
+ });
+
+ it('rejects sibling paths with the same prefix as the configured browse root', async () => {
+ const parent = await makeTempRoot();
+ const root = join(parent, 'books');
+ const sibling = join(parent, 'bookshelf');
+ await mkdir(root);
+ await mkdir(sibling);
+ const service = makeService(root);
+
+ await expect(service.assertWithinBrowseRoot(sibling)).rejects.toBeInstanceOf(ForbiddenException);
+ await expect(service.isWithinBrowseRoot(sibling)).resolves.toBe(false);
+ });
+
+ it('rejects paths that escape through a symlinked existing ancestor', async () => {
+ const parent = await makeTempRoot();
+ const root = join(parent, 'books');
+ const outside = join(parent, 'outside');
+ await mkdir(root);
+ await mkdir(outside);
+ await symlink(outside, join(root, 'escape'));
+ const service = makeService(root);
+
+ await expect(service.assertWithinBrowseRoot(join(root, 'escape', 'new-library'))).rejects.toBeInstanceOf(ForbiddenException);
+ });
+
+ async function makeTempRoot(): Promise {
+ const path = await mkdtemp(join(tmpdir(), 'bookorbit-path-policy-'));
+ tempRoots.push(path);
+ return resolve(path);
+ }
+
+ function makeService(root: string): PathPolicyService {
+ return new PathPolicyService({ libraryBrowseRoot: root } as never);
+ }
+});
diff --git a/server/src/modules/path/path-policy.service.ts b/server/src/modules/path/path-policy.service.ts
new file mode 100644
index 000000000..71459e199
--- /dev/null
+++ b/server/src/modules/path/path-policy.service.ts
@@ -0,0 +1,78 @@
+import type { ConfigType } from '@nestjs/config';
+import { Inject, Injectable, ForbiddenException } from '@nestjs/common';
+import { realpath } from 'fs/promises';
+import { basename, dirname, isAbsolute, join, relative, resolve } from 'path';
+
+import { storageConfig } from '../../config/config';
+
+@Injectable()
+export class PathPolicyService {
+ private readonly browseRoot: string;
+ private canonicalBrowseRootPromise: Promise | null = null;
+
+ constructor(@Inject(storageConfig.KEY) storage: ConfigType) {
+ this.browseRoot = resolve(storage.libraryBrowseRoot);
+ }
+
+ getBrowseRoot(): string {
+ return this.browseRoot;
+ }
+
+ async resolveBrowsePath(rawPath?: string | null): Promise {
+ const path = resolve(rawPath?.trim() || this.browseRoot);
+ await this.assertWithinBrowseRoot(path);
+ return path;
+ }
+
+ async assertWithinBrowseRoot(rawPath: string): Promise {
+ const path = resolve(rawPath);
+ if (!(await this.isWithinBrowseRoot(path))) {
+ throw new ForbiddenException('Path is outside the configured library browse root');
+ }
+ return path;
+ }
+
+ async isWithinBrowseRoot(rawPath: string): Promise {
+ const [root, path] = await Promise.all([this.getCanonicalBrowseRoot(), canonicalizeWithExistingAncestor(resolve(rawPath))]);
+ return isSameOrChildPath(root, path);
+ }
+
+ private getCanonicalBrowseRoot(): Promise {
+ this.canonicalBrowseRootPromise ??= canonicalizeWithExistingAncestor(this.browseRoot);
+ return this.canonicalBrowseRootPromise;
+ }
+}
+
+async function canonicalizeWithExistingAncestor(resolvedPath: string): Promise {
+ const suffixSegments: string[] = [];
+ let current = resolvedPath;
+
+ while (true) {
+ try {
+ const canonical = await realpath(current); // codeql[js/path-injection]
+ return suffixSegments.reduceRight((built, segment) => join(built, segment), canonical);
+ } catch (error) {
+ const code = getErrorCode(error);
+ if (code !== 'ENOENT' && code !== 'ENOTDIR') {
+ return resolvedPath;
+ }
+ }
+
+ const parent = dirname(current);
+ if (parent === current) return resolvedPath;
+
+ suffixSegments.push(basename(current));
+ current = parent;
+ }
+}
+
+function isSameOrChildPath(root: string, path: string): boolean {
+ const rel = relative(root, path);
+ return rel === '' || (!rel.startsWith('..') && !isAbsolute(rel));
+}
+
+function getErrorCode(error: unknown): string | undefined {
+ if (typeof error !== 'object' || error === null || !('code' in error)) return undefined;
+ const code = (error as { code?: unknown }).code;
+ return typeof code === 'string' ? code : undefined;
+}
diff --git a/server/src/modules/path/path.controller.test.ts b/server/src/modules/path/path.controller.test.ts
index 6d85562b3..6a861b856 100644
--- a/server/src/modules/path/path.controller.test.ts
+++ b/server/src/modules/path/path.controller.test.ts
@@ -2,6 +2,7 @@ import { PathController } from './path.controller';
describe('PathController', () => {
const pathService = {
+ getConfig: vi.fn(),
listDirectories: vi.fn(),
createDirectory: vi.fn(),
};
@@ -12,10 +13,16 @@ describe('PathController', () => {
vi.resetAllMocks();
});
- it('defaults to root path when query path is missing', async () => {
+ it('returns path config from the service', () => {
+ pathService.getConfig.mockReturnValue({ root: '/books' });
+
+ expect(controller.getConfig()).toEqual({ root: '/books' });
+ });
+
+ it('lets the service apply the default path when query path is missing', async () => {
await controller.listDirectories(undefined as never);
- expect(pathService.listDirectories).toHaveBeenCalledWith('/');
+ expect(pathService.listDirectories).toHaveBeenCalledWith(undefined);
});
it('passes provided query path through to the service', async () => {
diff --git a/server/src/modules/path/path.controller.ts b/server/src/modules/path/path.controller.ts
index 88b25d417..4a6fc3339 100644
--- a/server/src/modules/path/path.controller.ts
+++ b/server/src/modules/path/path.controller.ts
@@ -1,3 +1,4 @@
+import type { PathConfig } from '@bookorbit/types';
import { Permission } from '@bookorbit/types';
import { Body, Controller, Get, Post, Query } from '@nestjs/common';
@@ -9,10 +10,16 @@ import { PathService } from './path.service';
export class PathController {
constructor(private readonly pathService: PathService) {}
+ @Get('config')
+ @RequirePermission(Permission.ManageLibraries)
+ getConfig(): PathConfig {
+ return this.pathService.getConfig();
+ }
+
@Get()
@RequirePermission(Permission.ManageLibraries)
- listDirectories(@Query('path') path: string) {
- return this.pathService.listDirectories(path || '/');
+ listDirectories(@Query('path') path?: string) {
+ return this.pathService.listDirectories(path);
}
@Post()
diff --git a/server/src/modules/path/path.module.test.ts b/server/src/modules/path/path.module.test.ts
index 6998b8c1c..fbb82f444 100644
--- a/server/src/modules/path/path.module.test.ts
+++ b/server/src/modules/path/path.module.test.ts
@@ -2,11 +2,13 @@ import 'reflect-metadata';
import { PathController } from './path.controller';
import { PathModule } from './path.module';
+import { PathPolicyService } from './path-policy.service';
import { PathService } from './path.service';
describe('PathModule', () => {
it('registers path controller and service', () => {
expect(Reflect.getMetadata('controllers', PathModule)).toEqual([PathController]);
- expect(Reflect.getMetadata('providers', PathModule)).toEqual([PathService]);
+ expect(Reflect.getMetadata('providers', PathModule)).toEqual([PathPolicyService, PathService]);
+ expect(Reflect.getMetadata('exports', PathModule)).toEqual([PathPolicyService]);
});
});
diff --git a/server/src/modules/path/path.module.ts b/server/src/modules/path/path.module.ts
index f260a8114..d667e7cff 100644
--- a/server/src/modules/path/path.module.ts
+++ b/server/src/modules/path/path.module.ts
@@ -1,10 +1,12 @@
import { Module } from '@nestjs/common';
import { PathController } from './path.controller';
+import { PathPolicyService } from './path-policy.service';
import { PathService } from './path.service';
@Module({
controllers: [PathController],
- providers: [PathService],
+ providers: [PathPolicyService, PathService],
+ exports: [PathPolicyService],
})
export class PathModule {}
diff --git a/server/src/modules/path/path.service.test.ts b/server/src/modules/path/path.service.test.ts
index ef483e003..cc7c0af9a 100644
--- a/server/src/modules/path/path.service.test.ts
+++ b/server/src/modules/path/path.service.test.ts
@@ -2,6 +2,7 @@ vi.mock('fs/promises', () => ({
readdir: vi.fn(),
lstat: vi.fn(),
mkdir: vi.fn(),
+ realpath: vi.fn(),
}));
import { BadRequestException, ConflictException, ForbiddenException } from '@nestjs/common';
@@ -26,10 +27,23 @@ function entry(name: string, options: { directory?: boolean; symbolicLink?: bool
}
describe('PathService', () => {
- const service = new PathService();
+ const pathPolicy = {
+ getBrowseRoot: vi.fn(() => '/books'),
+ resolveBrowsePath: vi.fn((path?: string) => Promise.resolve(path || '/books')),
+ assertWithinBrowseRoot: vi.fn((path: string) => Promise.resolve(path)),
+ };
+ let service: PathService;
beforeEach(() => {
vi.resetAllMocks();
+ pathPolicy.getBrowseRoot.mockReturnValue('/books');
+ pathPolicy.resolveBrowsePath.mockImplementation((path?: string) => Promise.resolve(path || '/books'));
+ pathPolicy.assertWithinBrowseRoot.mockImplementation((path: string) => Promise.resolve(path));
+ service = new PathService(pathPolicy as never);
+ });
+
+ it('returns the configured browse root in config', () => {
+ expect(service.getConfig()).toEqual({ root: '/books' });
});
it('returns empty for blocked system paths', async () => {
@@ -39,6 +53,16 @@ describe('PathService', () => {
expect(readdirMock).not.toHaveBeenCalled();
});
+ it('uses the configured browse root when listing without a path', async () => {
+ lstatMock.mockResolvedValue({ isSymbolicLink: () => false, isDirectory: () => true } as never);
+ readdirMock.mockResolvedValue([] as never);
+
+ await expect(service.listDirectories()).resolves.toEqual([]);
+
+ expect(pathPolicy.resolveBrowsePath).toHaveBeenCalledWith(undefined);
+ expect(lstatMock).toHaveBeenCalledWith('/books');
+ });
+
it('rejects symlinked root paths', async () => {
lstatMock.mockResolvedValue({ isSymbolicLink: () => true, isDirectory: () => false } as never);
@@ -129,6 +153,14 @@ describe('PathService', () => {
expect(mkdirMock).not.toHaveBeenCalled();
});
+ it('rejects parent paths outside the configured browse root', async () => {
+ pathPolicy.resolveBrowsePath.mockRejectedValue(new ForbiddenException('outside root'));
+
+ await expect(service.createDirectory('/tmp/books', 'scifi')).rejects.toBeInstanceOf(ForbiddenException);
+ expect(lstatMock).not.toHaveBeenCalled();
+ expect(mkdirMock).not.toHaveBeenCalled();
+ });
+
it('rejects a symlinked parent', async () => {
lstatMock.mockResolvedValue({ isSymbolicLink: () => true, isDirectory: () => false } as never);
diff --git a/server/src/modules/path/path.service.ts b/server/src/modules/path/path.service.ts
index f9ea7d824..0e619b702 100644
--- a/server/src/modules/path/path.service.ts
+++ b/server/src/modules/path/path.service.ts
@@ -1,4 +1,4 @@
-import type { CreateFolderResult, DirectoryEntry } from '@bookorbit/types';
+import type { CreateFolderResult, DirectoryEntry, PathConfig } from '@bookorbit/types';
import {
BadRequestException,
ConflictException,
@@ -12,6 +12,7 @@ import { lstat, mkdir, readdir } from 'fs/promises';
import { join, resolve, sep } from 'path';
import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils';
+import { PathPolicyService } from './path-policy.service';
const BLOCKED = ['/proc', '/sys', '/dev', '/run', '/var/run', '/etc', '/root'];
@@ -19,8 +20,14 @@ const BLOCKED = ['/proc', '/sys', '/dev', '/run', '/var/run', '/etc', '/root'];
export class PathService {
private readonly logger = new Logger(PathService.name);
- async listDirectories(rawPath: string): Promise {
- const resolved = resolve(rawPath || '/');
+ constructor(private readonly pathPolicy: PathPolicyService) {}
+
+ getConfig(): PathConfig {
+ return { root: this.pathPolicy.getBrowseRoot() };
+ }
+
+ async listDirectories(rawPath?: string): Promise {
+ const resolved = await this.pathPolicy.resolveBrowsePath(rawPath);
if (this.isBlocked(resolved)) {
return [];
}
@@ -49,7 +56,7 @@ export class PathService {
async createDirectory(rawParent: string, rawName: string): Promise {
const name = this.assertSafeName(rawName);
- const parent = resolve(rawParent || '/');
+ const parent = await this.pathPolicy.resolveBrowsePath(rawParent);
if (this.isBlocked(parent)) {
throw new ForbiddenException('Cannot create a folder in this location');
}
@@ -63,9 +70,11 @@ export class PathService {
const target = join(parent, name);
const prefix = parent.endsWith(sep) ? parent : parent + sep;
- if (!resolve(target).startsWith(prefix)) {
+ const resolvedTarget = resolve(target);
+ if (!resolvedTarget.startsWith(prefix)) {
throw new BadRequestException('Invalid folder name');
}
+ await this.pathPolicy.assertWithinBrowseRoot(resolvedTarget);
await mkdir(target); // codeql[js/path-injection]
this.logger.log(`[path.create_folder] [end] path="${sanitizeLogValue(target)}" durationMs=${Date.now() - startedAt} - create folder completed`);
diff --git a/server/test/e2e/authorization-matrix/route-inventory/manifest.json b/server/test/e2e/authorization-matrix/route-inventory/manifest.json
index d8853084c..9bafd0e7d 100644
--- a/server/test/e2e/authorization-matrix/route-inventory/manifest.json
+++ b/server/test/e2e/authorization-matrix/route-inventory/manifest.json
@@ -1,5 +1,5 @@
{
- "totalRoutes": 353,
+ "totalRoutes": 354,
"byPermission": {
"(none)": 161,
"Permission.ManageMetadataConfig": 32,
@@ -7,7 +7,7 @@
"Permission.LibraryEditMetadata": 31,
"Permission.BookDockAccess": 17,
"Permission.ManageAppSettings": 23,
- "Permission.ManageLibraries": 13,
+ "Permission.ManageLibraries": 14,
"Permission.ManageUsers": 10,
"Permission.ManageEmail": 8,
"Permission.KoboSync": 7,
diff --git a/server/test/e2e/authorization-matrix/route-inventory/routes-05.json b/server/test/e2e/authorization-matrix/route-inventory/routes-05.json
index 939e4a6b8..99bd4d095 100644
--- a/server/test/e2e/authorization-matrix/route-inventory/routes-05.json
+++ b/server/test/e2e/authorization-matrix/route-inventory/routes-05.json
@@ -407,6 +407,18 @@
"allowDefault": false,
"useGuards": ["OpdsEnabledGuard, OpdsAuthGuard"]
},
+ {
+ "file": "server/src/modules/path/path.controller.ts",
+ "className": "PathController",
+ "methodName": "getConfig",
+ "httpMethod": "GET",
+ "path": "/path/config",
+ "permissions": ["Permission.ManageLibraries"],
+ "libraryAccess": [],
+ "isPublic": false,
+ "allowDefault": false,
+ "useGuards": []
+ },
{
"file": "server/src/modules/path/path.controller.ts",
"className": "PathController",