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
This commit is contained in:
neon
2026-07-07 17:02:46 +00:00
parent 0b11583f7d
commit 39d9be30f9
25 changed files with 515 additions and 43 deletions
+4
View File
@@ -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
+2
View File
@@ -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
@@ -1,7 +1,7 @@
<script setup lang="ts">
import { ref, computed, watch, nextTick } from 'vue'
import { ref, computed, watch, nextTick, onMounted } from 'vue'
import { Folder, FolderOpen, FolderPlus, ChevronRight, ChevronUp, Search, X, Check, Loader2, HardDrive } from '@lucide/vue'
import type { CreateFolderResult } from '@bookorbit/types'
import type { CreateFolderResult, PathConfig } from '@bookorbit/types'
import { api } from '@/lib/api'
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
@@ -15,9 +15,11 @@ const emit = defineEmits<{
close: []
}>()
const browseRoot = ref('/')
const currentPath = ref('/')
const entries = ref<DirEntry[]>([])
const loading = ref(false)
const initialized = ref(false)
const search = ref('')
const error = ref<string | null>(null)
@@ -27,28 +29,35 @@ const filteredEntries = computed(() => {
})
const breadcrumbs = computed(() => {
if (currentPath.value === '/') return [{ label: '/', path: '/' }]
const parts = currentPath.value.split('/').filter(Boolean)
const crumbs = [{ label: '/', path: '/' }]
let built = ''
const root = normalizePath(browseRoot.value)
const current = normalizePath(currentPath.value)
const crumbs = [{ label: root, path: root }]
if (current === root) return crumbs
const relative = root === '/' ? current.slice(1) : current.slice(root.length + 1)
const parts = relative.split('/').filter(Boolean)
let built = root === '/' ? '' : root
for (const part of parts) {
built += '/' + part
built = built ? `${built}/${part}` : `/${part}`
crumbs.push({ label: part, path: built })
}
return crumbs
})
const canGoUp = computed(() => currentPath.value !== '/')
const canGoUp = computed(() => normalizePath(currentPath.value) !== normalizePath(browseRoot.value))
async function navigate(path: string) {
search.value = ''
currentPath.value = path
const normalized = normalizePath(path)
currentPath.value = isAtOrBelowBrowseRoot(normalized) ? normalized : browseRoot.value
}
async function goUp() {
if (!canGoUp.value) return
const parts = currentPath.value.split('/').filter(Boolean)
parts.pop()
navigate(parts.length === 0 ? '/' : '/' + parts.join('/'))
const parent = parts.length === 0 ? '/' : '/' + parts.join('/')
navigate(parent)
}
async function loadEntries(path: string) {
@@ -70,7 +79,29 @@ async function loadEntries(path: string) {
}
}
watch(currentPath, (p) => loadEntries(p), { immediate: true })
async function loadConfig() {
try {
const res = await api('/api/v1/path/config')
if (res.ok) {
const config: PathConfig = await res.json()
browseRoot.value = normalizePath(config.root)
}
} catch {
browseRoot.value = '/'
} finally {
currentPath.value = browseRoot.value
await loadEntries(currentPath.value)
initialized.value = true
}
}
watch(currentPath, (p) => {
if (initialized.value) void loadEntries(p)
})
onMounted(() => {
void loadConfig()
})
function selectCurrent() {
emit('select', currentPath.value)
@@ -138,6 +169,17 @@ function onNewFolderKeydown(e: KeyboardEvent) {
cancelNewFolder()
}
}
function normalizePath(path: string): string {
const trimmed = path.trim() || '/'
if (trimmed === '/') return '/'
return trimmed.replace(/\/+$/, '')
}
function isAtOrBelowBrowseRoot(path: string): boolean {
const root = normalizePath(browseRoot.value)
return root === '/' || path === root || path.startsWith(`${root}/`)
}
</script>
<template>
@@ -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<Response>>())
vi.mock('@/lib/api', () => ({
api: apiMock,
}))
const tooltipStubs = {
Teleport: true,
Tooltip: { template: '<div><slot /></div>' },
TooltipTrigger: { template: '<div><slot /></div>' },
TooltipContent: { template: '<div><slot /></div>' },
}
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<void> }).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<typeof mountPicker>, 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<typeof mountPicker>, 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' },
})
}
+1
View File
@@ -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
+10 -9
View File
@@ -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` |
---
+4
View File
@@ -3,6 +3,10 @@ export interface DirectoryEntry {
path: string;
}
export interface PathConfig {
root: string;
}
export interface CreateFolderRequest {
parentPath: string;
name: string;
+4
View File
@@ -22,6 +22,10 @@ SETUP_BOOTSTRAP_TOKEN=
# ../local/data points to <project-root>/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
+15
View File
@@ -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 = '';
+2
View File
@@ -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 || '/'),
};
});
+18
View File
@@ -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();
});
});
+4
View File
@@ -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(),
+10 -1
View File
@@ -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],
@@ -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<ReturnType<typeof stat>>);
mockReaddir.mockResolvedValue([] as unknown as Awaited<ReturnType<typeof readdir>>);
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<ReturnType<typeof stat>>);
+25 -7
View File
@@ -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<string>('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<ReturnType<typeof stat>> | 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<string[]> {
return Promise.all(paths.map((path) => this.pathPolicy.assertWithinBrowseRoot(path)));
}
private async cleanupCoverDirectories(bookIds: number[]): Promise<void> {
const event = 'library.remove_cover_dirs';
const startedAt = Date.now();
@@ -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<string> {
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);
}
});
@@ -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<string> | null = null;
constructor(@Inject(storageConfig.KEY) storage: ConfigType<typeof storageConfig>) {
this.browseRoot = resolve(storage.libraryBrowseRoot);
}
getBrowseRoot(): string {
return this.browseRoot;
}
async resolveBrowsePath(rawPath?: string | null): Promise<string> {
const path = resolve(rawPath?.trim() || this.browseRoot);
await this.assertWithinBrowseRoot(path);
return path;
}
async assertWithinBrowseRoot(rawPath: string): Promise<string> {
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<boolean> {
const [root, path] = await Promise.all([this.getCanonicalBrowseRoot(), canonicalizeWithExistingAncestor(resolve(rawPath))]);
return isSameOrChildPath(root, path);
}
private getCanonicalBrowseRoot(): Promise<string> {
this.canonicalBrowseRootPromise ??= canonicalizeWithExistingAncestor(this.browseRoot);
return this.canonicalBrowseRootPromise;
}
}
async function canonicalizeWithExistingAncestor(resolvedPath: string): Promise<string> {
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;
}
@@ -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 () => {
+9 -2
View File
@@ -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()
+3 -1
View File
@@ -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]);
});
});
+3 -1
View File
@@ -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 {}
+33 -1
View File
@@ -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);
+14 -5
View File
@@ -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<DirectoryEntry[]> {
const resolved = resolve(rawPath || '/');
constructor(private readonly pathPolicy: PathPolicyService) {}
getConfig(): PathConfig {
return { root: this.pathPolicy.getBrowseRoot() };
}
async listDirectories(rawPath?: string): Promise<DirectoryEntry[]> {
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<CreateFolderResult> {
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`);
@@ -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,
@@ -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",