security: remediate dependency and code scanning alerts

Upgrade vulnerable transitive dependencies and harden filesystem, URL, HTML, cryptographic, and input-processing boundaries identified by security scans.
This commit is contained in:
neon
2026-07-15 17:34:54 +00:00
parent 876e32654a
commit f6394a9b0c
46 changed files with 792 additions and 472 deletions
+1 -1
View File
@@ -4,4 +4,4 @@
Use GitHub's [private vulnerability reporting](https://github.com/bookorbit/bookorbit/security/advisories/new) to report security issues confidentially. Do not open public issues for security vulnerabilities.
Reports are typically acknowledged within a few days, and fixes are published as quickly as severity warrants.
We aim to acknowledge vulnerability reports within 3 business days and provide a status update within 14 days. Please allow up to 90 days for investigation, remediation, and coordinated disclosure before publishing details. Critical vulnerabilities may be disclosed and fixed sooner when users face active risk.
+51
View File
@@ -0,0 +1,51 @@
name: OpenSSF Scorecard
on:
branch_protection_rule:
push:
branches: [main]
schedule:
- cron: "30 1 * * 6"
workflow_dispatch:
concurrency:
group: scorecard-${{ github.ref }}
cancel-in-progress: true
permissions: {}
jobs:
analysis:
name: Scorecard analysis
runs-on: ubuntu-latest
timeout-minutes: 15
if: github.event.repository.default_branch == github.ref_name || github.event_name == 'branch_protection_rule'
permissions:
actions: read # Read workflow metadata used by Scorecard checks.
contents: read # Read the repository being analyzed.
id-token: write # Publish signed Scorecard results.
security-events: write # Upload the SARIF report to code scanning.
steps:
- name: Checkout
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6
with:
persist-credentials: false
- name: Run Scorecard
uses: ossf/scorecard-action@4eaacf0543bb3f2c246792bd56e8cdeffafb205a # v2.4.3
with:
results_file: results.sarif
results_format: sarif
publish_results: true
- name: Upload artifact
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
with:
name: scorecard-results
path: results.sarif
retention-days: 5
- name: Upload to code scanning
uses: github/codeql-action/upload-sarif@99df26d4f13ea111d4ec1a7dddef6063f76b97e9 # v4
with:
sarif_file: results.sarif
+1
View File
@@ -149,6 +149,7 @@ BookOrbit is available in English, German, Dutch, Brazilian Portuguese, and Slov
- **Questions and discussion:** [GitHub Discussions](https://github.com/bookorbit/bookorbit/discussions)
- **Bug reports:** [GitHub Issues](https://github.com/bookorbit/bookorbit/issues/new?template=bug_report.yml)
- **Feature requests:** [GitHub Issues](https://github.com/bookorbit/bookorbit/issues/new?template=feature_request.yml)
- **Security vulnerabilities:** Follow the private reporting process in the [Security Policy](.github/SECURITY.md).
---
@@ -18,6 +18,12 @@ describe('description-html', () => {
expect(html).toBe('<p>Hello</p>')
})
it('does not treat nested script-like input as visible markup', () => {
const html = sanitizeDescriptionHtml('<scr<script>ipt>alert(1)</scr</script>ipt>')
expect(html).not.toContain('<script')
})
it('normalizes empty editor output to null', () => {
expect(normalizeDescriptionHtml('<p><br></p>')).toBeNull()
expect(normalizeDescriptionHtml('<ul><li></li></ul>')).toBeNull()
@@ -16,11 +16,7 @@ export function normalizeDescriptionHtml(content: string | null | undefined): st
}
function hasVisibleDescriptionText(content: string): boolean {
const text = content
.replace(/<br\s*\/?>/gi, '')
.replace(/<[^>]*>/g, '')
.replace(/&nbsp;/gi, ' ')
.trim()
return text.length > 0
const template = document.createElement('template')
template.innerHTML = content
return (template.content.textContent ?? '').trim().length > 0
}
@@ -27,6 +27,17 @@ describe('renderChangelogMarkdown', () => {
expect(html).not.toContain('javascript:')
})
it('escapes quotes in markdown link destinations before creating attributes', () => {
const html = renderChangelogMarkdown('[safe](https://example.com/"onmouseover="alert(1))')
const template = document.createElement('template')
template.innerHTML = html
const anchor = template.content.querySelector('a')
expect(anchor).not.toBeNull()
expect(anchor!.getAttribute('onmouseover')).toBeNull()
expect(anchor!.getAttributeNames().sort()).toEqual(['href', 'rel', 'target'])
})
it('returns empty string for empty input', () => {
expect(renderChangelogMarkdown(null)).toBe('')
expect(renderChangelogMarkdown('')).toBe('')
@@ -36,7 +36,7 @@ DOMPurify.addHook('afterSanitizeAttributes', (node) => {
})
function escapeHtml(text: string): string {
return text.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;')
return text.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;').replace(/"/g, '&quot;').replace(/'/g, '&#39;')
}
function renderInline(text: string): string {
+5
View File
@@ -82,10 +82,15 @@
},
"pnpm": {
"overrides": {
"@xhmikosr/decompress": "^11.1.3",
"@esbuild-kit/core-utils>esbuild": "^0.25.12",
"esbuild": "^0.28.1",
"fastify": "^5.8.5",
"fast-uri": "^3.1.1",
"fast-xml-builder": "^1.1.7",
"form-data": "^4.0.6",
"piscina": "^4.9.3",
"qs": "^6.15.2",
"ws": "^8.21.0"
},
"packageExtensions": {
+7 -4
View File
@@ -142,10 +142,7 @@ function normalizeResolverOptions(options?: PathResolverOptions): Required<PathR
}
export function sanitizePathSegment(value: string, replacementCharacter: "_" | "-" = "_"): string {
let sanitized = value
.replace(INVALID_SEGMENT_CHARS_REGEX, replacementCharacter)
.trim()
.replace(/[. ]+$/g, "");
let sanitized = stripTrailingDotsAndSpaces(value.replace(INVALID_SEGMENT_CHARS_REGEX, replacementCharacter).trim());
if (!sanitized || sanitized === "." || sanitized === "..") {
sanitized = replacementCharacter;
}
@@ -158,6 +155,12 @@ export function sanitizePathSegment(value: string, replacementCharacter: "_" | "
return sanitized;
}
function stripTrailingDotsAndSpaces(value: string): string {
let end = value.length;
while (end > 0 && (value[end - 1] === "." || value[end - 1] === " ")) end -= 1;
return end === value.length ? value : value.slice(0, end);
}
function sanitizeResolutionValues(values: Record<string, string>, options: Required<PathResolverOptions>): Record<string, string> {
if (!options.sanitizeForCrossPlatform) return values;
+261 -281
View File
File diff suppressed because it is too large Load Diff
+1 -1
View File
@@ -102,7 +102,7 @@
"zod": "^4.4.3"
},
"devDependencies": {
"@eslint/eslintrc": "^3.3.5",
"@eslint/eslintrc": "^3.3.6",
"@eslint/js": "^10.0.1",
"@ffmpeg-installer/ffmpeg": "^1.1.0",
"@ffprobe-installer/ffprobe": "^2.1.2",
@@ -30,4 +30,21 @@ describe('contentDispositionHeader', () => {
`attachment; filename="ok-_-_.epub"; filename*=UTF-8''ok--.epub`,
);
});
it('bounds oversized untrusted filenames before encoding', () => {
const header = contentDispositionHeader('attachment', `${'a'.repeat(10_000)}.epub`, 'download');
const quotedFilename = header.match(/filename="([^"]+)"/)?.[1];
expect(quotedFilename).toHaveLength(1024);
expect(header.length).toBeLessThan(2200);
});
it('rejects non-string runtime values without iterating over an attacker-controlled length', () => {
const maliciousValue = { length: Number.MAX_SAFE_INTEGER };
expect(contentDispositionHeader('attachment', maliciousValue as never, 'download')).toBe(`attachment; filename="download"`);
expect(contentDispositionHeader('attachment', 'book.epub', maliciousValue as never)).toBe(
`attachment; filename="book.epub"; filename*=UTF-8''book.epub`,
);
});
});
@@ -1,21 +1,15 @@
type ContentDispositionType = 'attachment' | 'inline';
const MAX_HEADER_FILENAME_CODE_UNITS = 1024;
function stripLoneSurrogates(value: string): string {
let out = '';
for (let i = 0; i < value.length; i++) {
const code = value.charCodeAt(i);
if (code >= 0xd800 && code <= 0xdbff) {
const next = value.charCodeAt(i + 1);
if (next >= 0xdc00 && next <= 0xdfff) {
out += value[i] + value[i + 1];
i += 1;
}
continue;
}
if (code >= 0xdc00 && code <= 0xdfff) continue;
out += value[i];
}
return out;
return Array.from(value, (character) => {
const codePoint = character.codePointAt(0);
return codePoint !== undefined && codePoint >= 0xd800 && codePoint <= 0xdfff ? '' : character;
}).join('');
}
function boundedHeaderValue(value: unknown): string {
return typeof value === 'string' ? value.slice(0, MAX_HEADER_FILENAME_CODE_UNITS) : '';
}
function encodeFilenameStar(value: string): string | null {
@@ -29,9 +23,11 @@ function encodeFilenameStar(value: string): string | null {
}
export function contentDispositionHeader(type: ContentDispositionType, filename: string, fallbackFilename: string): string {
const asciiFallback = fallbackFilename.replace(/[^\x20-\x7E]|["\\]/g, '_') || 'download';
const asciiFilename = filename.replace(/[^\x20-\x7E]|["\\]/g, '_') || asciiFallback;
const encodedFilename = encodeFilenameStar(filename);
const boundedFilename = boundedHeaderValue(filename);
const boundedFallback = boundedHeaderValue(fallbackFilename);
const asciiFallback = boundedFallback.replace(/[^\x20-\x7E]|["\\]/g, '_') || 'download';
const asciiFilename = boundedFilename.replace(/[^\x20-\x7E]|["\\]/g, '_') || asciiFallback;
const encodedFilename = encodeFilenameStar(boundedFilename);
const disposition = `${type}; filename="${asciiFilename}"`;
return encodedFilename ? `${disposition}; filename*=UTF-8''${encodedFilename}` : disposition;
@@ -0,0 +1,15 @@
import { htmlToPlainText } from './html-to-text.utils';
describe('htmlToPlainText', () => {
it('extracts text, decodes entities, and removes executable element contents', () => {
expect(htmlToPlainText('<p>Hello &amp; <strong>world</strong></p><script>alert(1)</script>')).toBe('Hello & world');
});
it('removes entity-encoded tag-shaped markup without evaluating it', () => {
expect(htmlToPlainText('&lt;script src="evil.js"&gt;alert(1)&lt;/script&gt; safe')).toBe('alert(1) safe');
});
it('preserves meaningful block and break boundaries when requested', () => {
expect(htmlToPlainText('<p>First<br>line</p><p>Second</p>', { preserveLineBreaks: true })).toBe('First\nline\nSecond');
});
});
@@ -0,0 +1,47 @@
import { load } from 'cheerio';
type HtmlToTextOptions = {
preserveLineBreaks?: boolean;
};
const BLOCK_ELEMENTS =
'address,article,aside,blockquote,div,dl,fieldset,figcaption,figure,footer,form,h1,h2,h3,h4,h5,h6,header,hr,li,main,nav,ol,p,pre,section,table,tr,ul';
export function htmlToPlainText(html: string, options: HtmlToTextOptions = {}): string {
const $ = load(html, undefined, false);
$('script,style,noscript,template').remove();
if (options.preserveLineBreaks) {
$('br').replaceWith('\n');
$(BLOCK_ELEMENTS).each((_index, element) => {
$(element).append('\n');
});
}
const text = stripAngleBracketMarkup($.root().text());
if (!options.preserveLineBreaks) return text.replace(/\s+/g, ' ').trim();
return text
.replace(/\r\n?/g, '\n')
.replace(/[ \t]+\n/g, '\n')
.replace(/\n[ \t]+/g, '\n')
.replace(/\n{3,}/g, '\n\n')
.trim();
}
function stripAngleBracketMarkup(value: string): string {
let result = '';
let cursor = 0;
while (cursor < value.length) {
const start = value.indexOf('<', cursor);
if (start < 0) return result + value.slice(cursor);
result += value.slice(cursor, start);
const end = value.indexOf('>', start + 1);
if (end < 0) return result + value.slice(start).replace(/[<>]/g, '');
cursor = end + 1;
}
return result;
}
@@ -0,0 +1,25 @@
import { amazonOrigin, audibleApiOrigin, normalizeAmazonDomain, normalizeAudibleDomain } from './metadata-provider-hosts.utils';
describe('metadata provider hosts', () => {
it('resolves supported Amazon marketplaces to fixed HTTPS origins', () => {
expect(normalizeAmazonDomain(' AMAZON.CO.UK ')).toBe('amazon.co.uk');
expect(amazonOrigin('amazon.co.uk')).toBe('https://www.amazon.co.uk');
});
it('rejects arbitrary Amazon hosts', () => {
expect(normalizeAmazonDomain('localhost')).toBe('amazon.com');
expect(amazonOrigin('amazon.com@127.0.0.1')).toBe('https://www.amazon.com');
});
it('normalizes supported Audible host styles and rejects arbitrary suffixes', () => {
expect(normalizeAudibleDomain('https://api.audible.co.uk/path')).toBe('co.uk');
expect(audibleApiOrigin('audible.co.uk')).toBe('https://api.audible.co.uk');
expect(audibleApiOrigin('localhost')).toBe('https://api.audible.com');
});
it('bounds hostile Audible input and removes paths without a regular expression', () => {
expect(normalizeAudibleDomain(`https://api.audible.co.uk/${'/'.repeat(100_000)}`)).toBe('co.uk');
expect(normalizeAudibleDomain(`https://api.audible.com${'x'.repeat(10_000)}/path`)).toBe('com');
expect(normalizeAudibleDomain({ length: Number.MAX_SAFE_INTEGER } as never)).toBe('com');
});
});
@@ -0,0 +1,66 @@
const AMAZON_ORIGINS: Readonly<Record<string, string>> = {
'amazon.com': 'https://www.amazon.com',
'amazon.co.uk': 'https://www.amazon.co.uk',
'amazon.de': 'https://www.amazon.de',
'amazon.fr': 'https://www.amazon.fr',
'amazon.it': 'https://www.amazon.it',
'amazon.es': 'https://www.amazon.es',
'amazon.ca': 'https://www.amazon.ca',
'amazon.com.au': 'https://www.amazon.com.au',
'amazon.co.jp': 'https://www.amazon.co.jp',
'amazon.in': 'https://www.amazon.in',
'amazon.com.br': 'https://www.amazon.com.br',
'amazon.com.mx': 'https://www.amazon.com.mx',
'amazon.nl': 'https://www.amazon.nl',
'amazon.se': 'https://www.amazon.se',
'amazon.pl': 'https://www.amazon.pl',
'amazon.sg': 'https://www.amazon.sg',
'amazon.ae': 'https://www.amazon.ae',
'amazon.sa': 'https://www.amazon.sa',
'amazon.tr': 'https://www.amazon.tr',
};
const AUDIBLE_API_ORIGINS: Readonly<Record<string, string>> = {
com: 'https://api.audible.com',
'co.uk': 'https://api.audible.co.uk',
de: 'https://api.audible.de',
fr: 'https://api.audible.fr',
it: 'https://api.audible.it',
es: 'https://api.audible.es',
ca: 'https://api.audible.ca',
'com.au': 'https://api.audible.com.au',
'co.jp': 'https://api.audible.co.jp',
in: 'https://api.audible.in',
};
const MAX_PROVIDER_DOMAIN_LENGTH = 256;
function normalizeProviderDomainInput(value: string | undefined | null): string {
return typeof value === 'string' ? value.slice(0, MAX_PROVIDER_DOMAIN_LENGTH).trim().toLowerCase() : '';
}
export function normalizeAmazonDomain(value: string | undefined | null): string {
const normalized = normalizeProviderDomainInput(value);
return AMAZON_ORIGINS[normalized] ? normalized : 'amazon.com';
}
export function amazonOrigin(value: string | undefined | null): string {
return AMAZON_ORIGINS[normalizeAmazonDomain(value)] ?? AMAZON_ORIGINS['amazon.com'];
}
export function normalizeAudibleDomain(value: string | undefined | null): string {
let domain = normalizeProviderDomainInput(value);
if (domain.startsWith('https://')) domain = domain.slice('https://'.length);
else if (domain.startsWith('http://')) domain = domain.slice('http://'.length);
if (domain.startsWith('api.audible.')) domain = domain.slice('api.audible.'.length);
else if (domain.startsWith('audible.')) domain = domain.slice('audible.'.length);
const pathStart = domain.indexOf('/');
if (pathStart !== -1) domain = domain.slice(0, pathStart);
return AUDIBLE_API_ORIGINS[domain] ? domain : 'com';
}
export function audibleApiOrigin(value: string | undefined | null): string {
return AUDIBLE_API_ORIGINS[normalizeAudibleDomain(value)] ?? AUDIBLE_API_ORIGINS.com;
}
@@ -4695,6 +4695,9 @@ describe('BookService', () => {
await expect(service.renameFile(fileId, { filename: '../new.epub' }, user)).rejects.toThrow(BadRequestException);
await expect(service.renameFile(fileId, { filename: 'dir/new.epub' }, user)).rejects.toThrow(BadRequestException);
await expect(service.renameFile(fileId, { filename: 'bad\0name.epub' }, user)).rejects.toThrow(BadRequestException);
await expect(service.renameFile(fileId, { filename: `${'a'.repeat(256)}.epub` }, user)).rejects.toThrow(BadRequestException);
expect(rename).not.toHaveBeenCalled();
});
it('only updates db if filename is not provided or unchanged', async () => {
+12 -3
View File
@@ -1341,12 +1341,21 @@ export class BookService {
let newAbsolutePath = file.absolutePath;
if (dto.filename && dto.filename !== basename(file.absolutePath)) {
if (dto.filename.includes('/') || dto.filename.includes('\\')) {
throw new BadRequestException('Filename cannot contain path separators');
const safeFilename = basename(dto.filename);
if (
safeFilename !== dto.filename ||
safeFilename === '.' ||
safeFilename === '..' ||
safeFilename.includes('\\') ||
safeFilename.includes('\0') ||
Buffer.byteLength(safeFilename, 'utf8') > 255
) {
throw new BadRequestException('Filename is invalid');
}
newAbsolutePath = join(dirname(file.absolutePath), dto.filename);
newAbsolutePath = join(dirname(file.absolutePath), safeFilename);
if (newAbsolutePath !== file.absolutePath) {
try {
// codeql[js/path-injection] basename constrains the user-provided value to one validated filename segment.
await rename(file.absolutePath, newAbsolutePath);
} catch (err) {
throw new BadRequestException(`Failed to rename file on disk: ${err instanceof Error ? err.message : String(err)}`);
@@ -46,6 +46,12 @@ describe('sanitizeSvgIcon', () => {
expect(sanitized).not.toContain('style=');
});
it('rejects comments containing script-like markup', () => {
expect(() => sanitizeSvgIcon(Buffer.from('<svg viewBox="0 0 24 24"><!-- <script>alert(1)</script> --><path d="M0 0"/></svg>'))).toThrow(
BadRequestException,
);
});
it('rejects doctype subsets', () => {
expect(() => sanitizeSvgIcon(Buffer.from('<!DOCTYPE svg [ <!ENTITY xxe SYSTEM "file:///etc/passwd"> ]><svg viewBox="0 0 24 24"></svg>'))).toThrow(
BadRequestException,
@@ -198,7 +198,6 @@ function stripSafePreamble(value: string): string {
return value
.replace(/^\uFEFF/, '')
.replace(/<\?xml[^>]*\?>/gi, '')
.replace(/<!--[\s\S]*?-->/g, '')
.replace(/<!DOCTYPE[^>]*>/gi, '')
.trim();
}
@@ -2,6 +2,7 @@ import { XMLParser, XMLBuilder } from 'fast-xml-parser';
import type { BookWritePayload, BookWritePayloadKey } from '../../interfaces/book-write-payload.interface';
import { COMIC_INFO_MANAGED_NOTES_KEYS, COMIC_INFO_PROVIDER_ID_KEYS, COMIC_INFO_PROVIDER_WEB_URL_BUILDERS } from '../../file-write.constants';
import { htmlToPlainText } from '../../../../common/utils/html-to-text.utils';
type ComicInfoObject = Record<string, unknown>;
@@ -36,7 +37,7 @@ export function buildComicInfoXml(existingXml: string | null, payload: BookWrite
const info: ComicInfoObject = existingXml ? parseComicInfoXml(existingXml) : {};
setComicInfoField(info, fieldMask, 'title', 'Title', payload.title);
setComicInfoField(info, fieldMask, 'description', 'Summary', payload.description, stripHtml);
setComicInfoField(info, fieldMask, 'description', 'Summary', payload.description, htmlToPlainText);
setComicInfoField(info, fieldMask, 'publisher', 'Publisher', payload.publisher);
setComicInfoField(info, fieldMask, 'seriesName', 'Series', payload.seriesName);
setIssueNumberField(info, fieldMask, payload);
@@ -89,18 +90,6 @@ export function buildComicInfoXml(existingXml: string | null, payload: BookWrite
return `<?xml version="1.0" encoding="UTF-8"?>\n${xmlBody}`;
}
function stripHtml(html: string): string {
return html
.replace(/&amp;/g, '&')
.replace(/&lt;/g, '<')
.replace(/&gt;/g, '>')
.replace(/&quot;/g, '"')
.replace(/&#39;/g, "'")
.replace(/<[^>]*>/g, ' ')
.replace(/\s{2,}/g, ' ')
.trim();
}
function formatSeriesIndex(val: number): string {
return val % 1 === 0 ? String(Math.trunc(val)) : String(val);
}
@@ -1,5 +1,6 @@
import { MetadataCandidate, MetadataProviderKey } from '@bookorbit/types';
import { parsePublishedDateKey, parsePublishedYear, publishedYearFromDateKey } from '../../../../common/utils/published-date.utils';
import { htmlToPlainText } from '../../../../common/utils/html-to-text.utils';
import { AladinItem } from './aladin.types';
function parseAuthors(authorString: string): string[] {
@@ -22,7 +23,7 @@ function parsePageCount(subInfo?: AladinItem['subInfo']): number | undefined {
function parseDescription(description: string, fullDescription?: string): string | undefined {
const desc = fullDescription ?? description;
if (!desc) return undefined;
return desc.replace(/<[^>]*>/g, '').trim();
return htmlToPlainText(desc) || undefined;
}
function parseGenres(categoryIdList?: AladinItem['categoryIdList']): string[] | undefined {
@@ -3,6 +3,7 @@ import { MetadataCandidate, MetadataProviderKey } from '@bookorbit/types';
import { ProviderConfigService } from '../../../metadata-preferences/provider-config.service';
import { sanitizeLogValue } from '../../../../common/utils/log-sanitize.utils';
import { amazonOrigin } from '../../../../common/utils/metadata-provider-hosts.utils';
import { fetchWithThrottle } from '../../fetch-with-throttle';
import { ProviderThrottleError } from '../../provider-throttle.error';
import { IdentifiableProvider } from '../metadata-provider';
@@ -60,14 +61,14 @@ export class AmazonProvider implements IdentifiableProvider {
private async searchAsins(params: MetadataSearchParams, domain: string, cookie: string, limit: number, signal?: AbortSignal): Promise<string[]> {
const query = params.isbn?.trim() || [params.title, params.author].filter(Boolean).join(' ');
if (!query) return [];
const url = `https://www.${domain}/s?k=${encodeURIComponent(query)}&i=stripbooks`;
const url = `${amazonOrigin(domain)}/s?k=${encodeURIComponent(query)}&i=stripbooks`;
const html = await this.fetchHtml(url, cookie, 'search', query, undefined, signal);
return html ? extractAsins(html, limit) : [];
}
private async fetchByAsin(asin: string, domain: string, cookie: string, signal?: AbortSignal): Promise<MetadataCandidate | null> {
const url = `https://www.${domain}/dp/${asin}`;
const html = await this.fetchHtml(url, cookie, 'lookup', undefined, asin, signal);
const url = new URL(`/dp/${encodeURIComponent(asin)}`, amazonOrigin(domain));
const html = await this.fetchHtml(url.toString(), cookie, 'lookup', undefined, asin, signal);
if (!html) return null;
const data = parseBookPage(html);
if (!data.title) return null;
@@ -89,7 +90,7 @@ export class AmazonProvider implements IdentifiableProvider {
seriesIndex: data.seriesIndex,
coverUrl: data.coverUrl,
genres: data.tags?.length ? data.tags : undefined,
sourceUrl: `https://www.${domain}/dp/${asin}`,
sourceUrl: url.toString(),
...(data.communityRating !== undefined ? { communityRating: data.communityRating } : {}),
...(data.communityRatingCount !== undefined ? { communityRatingCount: data.communityRatingCount } : {}),
};
@@ -3,6 +3,7 @@ import { MetadataCandidate, MetadataProviderKey } from '@bookorbit/types';
import { ProviderConfigService } from '../../../metadata-preferences/provider-config.service';
import { sanitizeLogValue } from '../../../../common/utils/log-sanitize.utils';
import { audibleApiOrigin } from '../../../../common/utils/metadata-provider-hosts.utils';
import { fetchWithThrottle } from '../../fetch-with-throttle';
import { ProviderThrottleError } from '../../provider-throttle.error';
import { IdentifiableProvider } from '../metadata-provider';
@@ -31,7 +32,7 @@ export class AudibleProvider implements IdentifiableProvider {
const query = this.buildQuery(params);
if (!query) return [];
const url = new URL(`https://api.audible.${normalizedDomain}/1.0/catalog/products`);
const url = new URL('/1.0/catalog/products', audibleApiOrigin(normalizedDomain));
url.searchParams.set('num_results', '10');
url.searchParams.set('keywords', query);
url.searchParams.set('response_groups', 'product_desc,media,product_attrs,series,product_plan_details,category_ladders,rating');
@@ -71,7 +72,7 @@ export class AudibleProvider implements IdentifiableProvider {
if (!enabled) return null;
const normalizedDomain = normalizeAudibleDomain(domain);
const url = new URL(`https://api.audible.${normalizedDomain}/1.0/catalog/products/${providerId}`);
const url = new URL(`/1.0/catalog/products/${encodeURIComponent(providerId)}`, audibleApiOrigin(normalizedDomain));
url.searchParams.set('response_groups', 'product_desc,media,product_attrs,series,product_plan_details,category_ladders,rating');
const requestUrl = url.toString();
const startedAt = Date.now();
@@ -1,11 +1 @@
export function normalizeAudibleDomain(value: string | undefined | null): string {
let domain = (value ?? '').trim().toLowerCase();
if (!domain) return 'com';
domain = domain.replace(/^https?:\/\//, '');
domain = domain.replace(/^api\.audible\./, '');
domain = domain.replace(/^audible\./, '');
domain = domain.replace(/\/.*$/, '');
return domain || 'com';
}
export { normalizeAudibleDomain } from '../../../../common/utils/metadata-provider-hosts.utils';
@@ -64,7 +64,7 @@ describe('mapAudNexusBook — basic fields', () => {
it('strips HTML from summary when description is absent', () => {
const result = mapAudNexusBook(makeBook({ summary: '<p>A lone <b>astronaut</b>.</p>' }));
expect(result.description).toBe('A lone astronaut .');
expect(result.description).toBe('A lone astronaut.');
});
it('maps publisherName', () => {
@@ -8,6 +8,7 @@ import {
GoodreadsAutocompleteItem,
} from './goodreads.types';
import { parsePublishedDateFromEpochMillis, publishedYearFromDateKey } from '../../../../common/utils/published-date.utils';
import { htmlToPlainText } from '../../../../common/utils/html-to-text.utils';
export function mapGoodreadsApolloState(state: Record<string, unknown>, bookId: string): MetadataCandidate | null {
const book = findBook(state, bookId);
@@ -106,17 +107,7 @@ function parseSeriesFromTitle(title: string | undefined): { seriesName?: string;
function extractAutocompleteDescription(description: GoodreadsAutocompleteItem['description']): string | undefined {
const html = typeof description === 'string' ? description : description?.html;
if (!html) return undefined;
const text = html
.replace(/<br\s*\/?>/gi, '\n')
.replace(/<[^>]+>/g, '')
.replace(/&quot;/g, '"')
.replace(/&(?:#39|#x27|apos);/g, "'")
.replace(/&lt;/g, '<')
.replace(/&gt;/g, '>')
.replace(/&amp;/g, '&')
.replace(/[ \t]+\n/g, '\n')
.replace(/\n{3,}/g, '\n\n')
.trim();
const text = htmlToPlainText(html, { preserveLineBreaks: true });
return text || undefined;
}
@@ -1,10 +1,4 @@
const HTML_ENTITY_REPLACEMENTS: Array<[RegExp, string]> = [
[/&amp;/g, '&'],
[/&lt;/g, '<'],
[/&gt;/g, '>'],
[/&quot;/g, '"'],
[/&#39;/g, "'"],
];
import { htmlToPlainText } from '../../../common/utils/html-to-text.utils';
export function normalizeMaxCandidates(value: number | undefined, maxResults: number): number {
if (!Number.isFinite(value) || value == null) return maxResults;
@@ -42,11 +36,7 @@ export function sleep(ms: number, signal?: AbortSignal): Promise<void> {
}
export function stripHtml(html: string): string {
let value = html.replace(/<[^>]*>/g, ' ');
for (const [pattern, replacement] of HTML_ENTITY_REPLACEMENTS) {
value = value.replace(pattern, replacement);
}
return value.replace(/\s+/g, ' ').trim();
return htmlToPlainText(html);
}
export function sanitizeLogError(error: unknown): string {
@@ -488,6 +488,19 @@ describe('ProviderConfigService', () => {
expect(result.message).toContain('bot-check');
});
it('falls back to the allowlisted Amazon origin for an untrusted domain', async () => {
db.query.appSettings.findFirst.mockResolvedValue(undefined);
fetchMock.mockResolvedValue(new Response('<html>Amazon results</html>', { status: 200 }));
await service.testProvider(MetadataProviderKey.AMAZON, {
amazon: { domain: 'localhost' },
});
const requestUrl = fetchMock.mock.calls[0]?.[0];
expect(requestUrl).toBeInstanceOf(URL);
expect((requestUrl as URL).hostname).toBe('www.amazon.com');
});
it('rejects unsupported provider test requests', async () => {
await expect(service.testProvider(MetadataProviderKey.GOODREADS, {})).rejects.toThrow('Provider test not supported');
});
@@ -5,6 +5,7 @@ import { MetadataProviderKey, ProviderConfigurations, ProviderConnectionTestResu
import { stripBearerPrefix, toBearerAuthorization } from '../../common/utils/bearer-token.utils';
import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils';
import { amazonOrigin, normalizeAmazonDomain, normalizeAudibleDomain } from '../../common/utils/metadata-provider-hosts.utils';
import { DB } from '../../db';
import * as schema from '../../db/schema';
@@ -274,7 +275,7 @@ export class ProviderConfigService {
google: { ...config.google, apiKey: googleApiKey },
amazon: {
...config.amazon,
domain: this.normalizeDomain(config.amazon.domain, DEFAULT_CONFIG.amazon.domain),
domain: normalizeAmazonDomain(config.amazon.domain),
cookie: this.normalizeAmazonCookie(config.amazon.cookie),
},
hardcover: {
@@ -283,7 +284,7 @@ export class ProviderConfigService {
},
audible: {
...config.audible,
domain: this.normalizeDomain(config.audible.domain, DEFAULT_CONFIG.audible.domain),
domain: this.normalizeAudibleConfigDomain(config.audible.domain),
},
comicvine: {
...config.comicvine,
@@ -307,11 +308,6 @@ export class ProviderConfigService {
};
}
private normalizeDomain(value: string, fallback: string): string {
const normalized = value.trim().toLowerCase();
return normalized || fallback;
}
private normalizeAmazonCookie(cookie: string): string {
const normalized = cookie.trim();
if (!normalized) return '';
@@ -321,6 +317,13 @@ export class ProviderConfigService {
return normalized;
}
private normalizeAudibleConfigDomain(value: string): string {
const normalized = value.trim().toLowerCase();
const suffix = normalizeAudibleDomain(normalized);
if (normalized === suffix || normalized === `audible.${suffix}`) return normalized;
return DEFAULT_CONFIG.audible.domain;
}
private normalizeKoboPathSegment(value: string, fallback: string): string {
const normalized = value
.trim()
@@ -517,9 +520,11 @@ export class ProviderConfigService {
}
private async testAmazonProvider(config: ProviderConfigurations['amazon']): Promise<ProviderConnectionTestResult> {
const domain = this.normalizeDomain(config.domain, DEFAULT_CONFIG.amazon.domain);
const domain = normalizeAmazonDomain(config.domain);
const cookie = this.normalizeAmazonCookie(config.cookie);
const url = `https://www.${domain}/s?k=${encodeURIComponent(AMAZON_TEST_QUERY)}&i=stripbooks`;
const url = new URL('/s', amazonOrigin(domain));
url.searchParams.set('k', AMAZON_TEST_QUERY);
url.searchParams.set('i', 'stripbooks');
const headers: HeadersInit = cookie ? { ...AMAZON_TEST_HEADERS, cookie } : AMAZON_TEST_HEADERS;
const response = await fetch(url, { method: 'GET', headers, signal: AbortSignal.timeout(PROVIDER_TEST_TIMEOUT_MS) });
@@ -3,6 +3,7 @@ import { XMLParser } from 'fast-xml-parser';
import { attr, findInZip, toRecordArray } from './epub-zip-utils';
import { findIsbnInText, pickBestIsbn } from './isbn-detect';
import { htmlToPlainText } from '../../../common/utils/html-to-text.utils';
const SPINE_WINDOW_FRONT = 10;
const SPINE_WINDOW_BACK = 5;
@@ -22,13 +23,6 @@ interface IsbnResult {
const EMPTY: IsbnResult = { isbn10: null, isbn13: null };
function stripTags(html: string): string {
return html
.replace(/<script[\s\S]*?<\/script>/gi, ' ')
.replace(/<style[\s\S]*?<\/style>/gi, ' ')
.replace(/<[^>]+>/g, ' ');
}
async function readEntryCapped(file: unzipper.File): Promise<string> {
const size = file.uncompressedSize ?? 0;
if (size === 0 || size <= PER_DOC_BYTE_CAP) {
@@ -92,7 +86,7 @@ export async function scanEpubSpineForIsbn(zip: unzipper.CentralDirectory, opfXm
for (const doc of docs) {
let text: string;
try {
text = stripTags(await readEntryCapped(doc));
text = htmlToPlainText(await readEntryCapped(doc));
} catch {
continue;
}
@@ -1,5 +1,7 @@
import { readFile } from 'fs/promises';
import { htmlToPlainText } from '../../../common/utils/html-to-text.utils';
// ── PalmDB / MOBI binary offsets ──────────────────────────────────────────────
//
// PalmDB header (file start):
@@ -108,10 +110,6 @@ function exthUint32(records: ExthRecord[], type: number): number | null {
return rec.data.readUInt32BE(0);
}
function stripHtml(html: string): string {
return html.replace(/<[^>]*>/g, '').trim();
}
export function parseMobiBuffer(buf: Buffer): MobiParsed {
const recordOffsets = readRecords(buf);
if (recordOffsets.length === 0) throw new Error('No records in PalmDB');
@@ -146,7 +144,7 @@ export function parseMobiBuffer(buf: Buffer): MobiParsed {
const authors = exthStrings(exthRecords, EXTH.AUTHOR);
const publisher = exthString(exthRecords, EXTH.PUBLISHER);
const rawDescription = exthString(exthRecords, EXTH.DESCRIPTION);
const description = rawDescription ? stripHtml(rawDescription) : null;
const description = rawDescription ? htmlToPlainText(rawDescription) : null;
const isbn = exthString(exthRecords, EXTH.ISBN);
const tags = exthStrings(exthRecords, EXTH.SUBJECT);
const publishedDate = exthString(exthRecords, EXTH.PUBLISHED_DATE);
@@ -90,7 +90,7 @@ describe('UserStateImporter', () => {
],
readingSessions: [
{
sourceSessionId: 'session-1',
sourceSessionId: 'session-'.repeat(20),
sourceUserId: 'u1',
sourceBookId: 'b-source',
bookType: 'EPUB',
@@ -173,7 +173,7 @@ describe('UserStateImporter', () => {
userId: 10,
bookId: 200,
bookFileId: 500,
sessionId: 'booklore:rs:session-1',
sessionId: expect.stringMatching(/^booklore:rs:[A-Za-z0-9_-]{43}$/),
durationSeconds: 1800,
progressDelta: 12.35,
endProgress: 100,
@@ -774,7 +774,8 @@ function buildImportedReadingSessionId(sourceType: string, sourceSessionId: stri
const raw = sourceSessionId.trim();
const candidate = `${prefix}${raw}`;
if (candidate.length <= 64) return candidate;
return `${prefix}${createHash('sha1').update(raw).digest('hex')}`;
const digest = createHash('sha256').update(sourceType).update('\0').update(raw).digest('base64url');
return `${prefix}${digest.slice(0, 64 - prefix.length)}`;
}
function sanitizeNullablePercent(value: number | null): number | null {
@@ -1,4 +1,4 @@
import { ForbiddenException } from '@nestjs/common';
import { BadRequestException, ForbiddenException } from '@nestjs/common';
import { mkdtemp, mkdir, rm, symlink } from 'fs/promises';
import { tmpdir } from 'os';
import { join, resolve } from 'path';
@@ -43,6 +43,25 @@ describe('PathPolicyService', () => {
await expect(service.isWithinBrowseRoot(sibling)).resolves.toBe(false);
});
it('rejects normalized traversal paths before filesystem access', async () => {
const parent = await makeTempRoot();
const root = join(parent, 'books');
await mkdir(root);
const service = makeService(root);
await expect(service.assertWithinBrowseRoot(join(root, '..', 'outside'))).rejects.toBeInstanceOf(ForbiddenException);
await expect(service.isWithinBrowseRoot(join(root, '..', 'outside'))).resolves.toBe(false);
});
it('rejects oversized paths before resolving or accessing them', async () => {
const root = await makeTempRoot();
const service = makeService(root);
const oversizedPath = `/${'a'.repeat(4097)}`;
await expect(service.resolveBrowsePath(oversizedPath)).rejects.toBeInstanceOf(BadRequestException);
await expect(service.isWithinBrowseRoot(oversizedPath)).resolves.toBe(false);
});
it('rejects paths that escape through a symlinked existing ancestor', async () => {
const parent = await makeTempRoot();
const root = join(parent, 'books');
@@ -55,6 +74,15 @@ describe('PathPolicyService', () => {
await expect(service.assertWithinBrowseRoot(join(root, 'escape', 'new-library'))).rejects.toBeInstanceOf(ForbiddenException);
});
it('fails closed when a path cannot be canonicalized safely', async () => {
const root = await makeTempRoot();
await symlink('loop', join(root, 'loop'));
const service = makeService(root);
await expect(service.assertWithinBrowseRoot(join(root, 'loop'))).rejects.toBeInstanceOf(ForbiddenException);
await expect(service.isWithinBrowseRoot(join(root, 'loop'))).resolves.toBe(false);
});
async function makeTempRoot(): Promise<string> {
const path = await mkdtemp(join(tmpdir(), 'bookorbit-path-policy-'));
tempRoots.push(path);
+42 -11
View File
@@ -1,10 +1,12 @@
import type { ConfigType } from '@nestjs/config';
import { Inject, Injectable, ForbiddenException } from '@nestjs/common';
import { BadRequestException, ForbiddenException, Inject, Injectable } from '@nestjs/common';
import { realpath } from 'fs/promises';
import { basename, dirname, isAbsolute, join, relative, resolve } from 'path';
import { basename, dirname, isAbsolute, join, relative, resolve, sep } from 'path';
import { storageConfig } from '../../config/config';
const MAX_BROWSE_PATH_LENGTH = 4096;
@Injectable()
export class PathPolicyService {
private readonly browseRoot: string;
@@ -19,7 +21,10 @@ export class PathPolicyService {
}
async resolveBrowsePath(rawPath?: string | null): Promise<string> {
const path = resolve(rawPath?.trim() || this.browseRoot);
const inputPath = typeof rawPath === 'string' ? rawPath.trim() : '';
if (inputPath.length > MAX_BROWSE_PATH_LENGTH) throw new BadRequestException('Path is too long');
const path = resolve(inputPath || this.browseRoot);
await this.assertWithinBrowseRoot(path);
return path;
}
@@ -33,8 +38,39 @@ export class PathPolicyService {
}
async isWithinBrowseRoot(rawPath: string): Promise<boolean> {
const [root, path] = await Promise.all([this.getCanonicalBrowseRoot(), canonicalizeWithExistingAncestor(resolve(rawPath))]);
return isSameOrChildPath(root, path);
if (typeof rawPath !== 'string' || rawPath.length > MAX_BROWSE_PATH_LENGTH) return false;
const resolvedPath = resolve(rawPath);
const lexicalRelativePath = relative(this.browseRoot, resolvedPath);
if (lexicalRelativePath === '..' || lexicalRelativePath.startsWith(`..${sep}`) || isAbsolute(lexicalRelativePath)) return false;
const suffixSegments: string[] = [];
let current = resolvedPath;
let canonicalPath: string;
while (true) {
try {
const canonicalAncestor = await realpath(current);
canonicalPath = suffixSegments.reduceRight((built, segment) => join(built, segment), canonicalAncestor);
break;
} catch (error) {
const code = getErrorCode(error);
if (code !== 'ENOENT' && code !== 'ENOTDIR') return false;
}
const parent = dirname(current);
if (parent === current) return false;
suffixSegments.push(basename(current));
current = parent;
}
const root = await this.getCanonicalBrowseRoot();
const canonicalRelativePath = relative(root, canonicalPath);
return (
canonicalRelativePath === '' ||
(!canonicalRelativePath.startsWith(`..${sep}`) && canonicalRelativePath !== '..' && !isAbsolute(canonicalRelativePath))
);
}
private getCanonicalBrowseRoot(): Promise<string> {
@@ -49,7 +85,7 @@ async function canonicalizeWithExistingAncestor(resolvedPath: string): Promise<s
while (true) {
try {
const canonical = await realpath(current); // codeql[js/path-injection]
const canonical = await realpath(current);
return suffixSegments.reduceRight((built, segment) => join(built, segment), canonical);
} catch (error) {
const code = getErrorCode(error);
@@ -66,11 +102,6 @@ async function canonicalizeWithExistingAncestor(resolvedPath: string): Promise<s
}
}
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;
+37 -34
View File
@@ -28,22 +28,22 @@ function entry(name: string, options: { directory?: boolean; symbolicLink?: bool
describe('PathService', () => {
const pathPolicy = {
getBrowseRoot: vi.fn(() => '/books'),
resolveBrowsePath: vi.fn((path?: string) => Promise.resolve(path || '/books')),
getBrowseRoot: vi.fn(() => '/tmp/books'),
resolveBrowsePath: vi.fn((path?: string) => Promise.resolve(path || '/tmp/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.getBrowseRoot.mockReturnValue('/tmp/books');
pathPolicy.resolveBrowsePath.mockImplementation((path?: string) => Promise.resolve(path || '/tmp/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' });
expect(service.getConfig()).toEqual({ root: '/tmp/books' });
});
it('returns empty for blocked system paths', async () => {
@@ -60,7 +60,7 @@ describe('PathService', () => {
await expect(service.listDirectories()).resolves.toEqual([]);
expect(pathPolicy.resolveBrowsePath).toHaveBeenCalledWith(undefined);
expect(lstatMock).toHaveBeenCalledWith('/books');
expect(lstatMock).toHaveBeenCalledWith('/tmp/books');
});
it('rejects symlinked root paths', async () => {
@@ -70,42 +70,27 @@ describe('PathService', () => {
});
it('lists only accessible real (non-symlink) directories and sorts them by name', async () => {
lstatMock.mockImplementation((fullPath) => {
if (String(fullPath) === '/tmp/books') {
return Promise.resolve({ isSymbolicLink: () => false, isDirectory: () => true } as never);
}
if (String(fullPath).endsWith('/beta')) {
throw Object.assign(new Error('denied'), { code: 'EACCES' });
}
if (String(fullPath).endsWith('/alpha-link')) {
return Promise.resolve({ isDirectory: () => true, isSymbolicLink: () => true } as never);
}
return Promise.resolve({ isDirectory: () => true, isSymbolicLink: () => false } as never);
});
lstatMock.mockResolvedValue({ isSymbolicLink: () => false, isDirectory: () => true } as never);
readdirMock.mockResolvedValue([
entry('notes.txt'),
entry('.cache', { directory: true }),
entry('zeta', { directory: true }),
entry('alpha-link', { directory: true }),
entry('alpha-link', { directory: true, symbolicLink: true }),
entry('beta', { directory: true }),
] as never);
await expect(service.listDirectories('/tmp/books')).resolves.toEqual([{ name: 'zeta', path: '/tmp/books/zeta' }]);
await expect(service.listDirectories('/tmp/books')).resolves.toEqual([
{ name: 'beta', path: '/tmp/books/beta' },
{ name: 'zeta', path: '/tmp/books/zeta' },
]);
expect(lstatMock).toHaveBeenCalledTimes(1);
});
it('excludes entries that appear as directories but are symlinks when lstat confirms it', async () => {
lstatMock.mockImplementation((fullPath) => {
if (String(fullPath) === '/tmp/books') {
return Promise.resolve({ isSymbolicLink: () => false, isDirectory: () => true } as never);
}
if (String(fullPath).endsWith('/real-dir')) {
return Promise.resolve({ isDirectory: () => true, isSymbolicLink: () => false } as never);
}
return Promise.resolve({ isDirectory: () => true, isSymbolicLink: () => true } as never);
});
lstatMock.mockResolvedValue({ isSymbolicLink: () => false, isDirectory: () => true } as never);
readdirMock.mockResolvedValue([entry('real-dir', { directory: true }), entry('sym-dir', { directory: true })] as never);
readdirMock.mockResolvedValue([entry('real-dir', { directory: true }), entry('sym-dir', { directory: true, symbolicLink: true })] as never);
await expect(service.listDirectories('/tmp/books')).resolves.toEqual([{ name: 'real-dir', path: '/tmp/books/real-dir' }]);
});
@@ -121,7 +106,16 @@ describe('PathService', () => {
lstatMock.mockResolvedValue({ isSymbolicLink: () => false, isDirectory: () => true } as never);
readdirMock.mockRejectedValue(new Error('missing'));
await expect(service.listDirectories('/does/not/exist')).resolves.toEqual([]);
await expect(service.listDirectories('/tmp/books/does/not/exist')).resolves.toEqual([]);
});
it('rechecks browse-root containment before accessing the filesystem', async () => {
pathPolicy.getBrowseRoot.mockReturnValue('/books');
pathPolicy.resolveBrowsePath.mockResolvedValue('/etc');
await expect(service.listDirectories('/books/../../etc')).resolves.toEqual([]);
expect(lstatMock).not.toHaveBeenCalled();
expect(readdirMock).not.toHaveBeenCalled();
});
describe('createDirectory', () => {
@@ -164,14 +158,14 @@ describe('PathService', () => {
it('rejects a symlinked parent', async () => {
lstatMock.mockResolvedValue({ isSymbolicLink: () => true, isDirectory: () => false } as never);
await expect(service.createDirectory('/tmp/link', 'scifi')).rejects.toBeInstanceOf(ForbiddenException);
await expect(service.createDirectory('/tmp/books/link', 'scifi')).rejects.toBeInstanceOf(ForbiddenException);
expect(mkdirMock).not.toHaveBeenCalled();
});
it('rejects a parent that is not a directory', async () => {
lstatMock.mockResolvedValue({ isSymbolicLink: () => false, isDirectory: () => false } as never);
await expect(service.createDirectory('/tmp/file.txt', 'scifi')).rejects.toBeInstanceOf(BadRequestException);
await expect(service.createDirectory('/tmp/books/file.txt', 'scifi')).rejects.toBeInstanceOf(BadRequestException);
expect(mkdirMock).not.toHaveBeenCalled();
});
@@ -205,7 +199,16 @@ describe('PathService', () => {
it('maps a missing parent (ENOENT) to a bad request', async () => {
lstatMock.mockRejectedValue(Object.assign(new Error('missing'), { code: 'ENOENT' }));
await expect(service.createDirectory('/tmp/gone', 'scifi')).rejects.toBeInstanceOf(BadRequestException);
await expect(service.createDirectory('/tmp/books/gone', 'scifi')).rejects.toBeInstanceOf(BadRequestException);
expect(mkdirMock).not.toHaveBeenCalled();
});
it('rechecks browse-root containment before accessing the filesystem', async () => {
pathPolicy.getBrowseRoot.mockReturnValue('/books');
pathPolicy.resolveBrowsePath.mockResolvedValue('/etc');
await expect(service.createDirectory('/books/../../etc', 'scifi')).rejects.toBeInstanceOf(ForbiddenException);
expect(lstatMock).not.toHaveBeenCalled();
expect(mkdirMock).not.toHaveBeenCalled();
});
});
+30 -22
View File
@@ -9,7 +9,7 @@ import {
Logger,
} from '@nestjs/common';
import { lstat, mkdir, readdir } from 'fs/promises';
import { join, resolve, sep } from 'path';
import { basename, isAbsolute, join, relative, resolve, sep } from 'path';
import { sanitizeLogValue } from '../../common/utils/log-sanitize.utils';
import { PathPolicyService } from './path-policy.service';
@@ -27,26 +27,25 @@ export class PathService {
}
async listDirectories(rawPath?: string): Promise<DirectoryEntry[]> {
const resolved = await this.pathPolicy.resolveBrowsePath(rawPath);
const browseRoot = resolve(this.pathPolicy.getBrowseRoot());
const resolved = resolve(await this.pathPolicy.resolveBrowsePath(rawPath));
const relativePath = relative(browseRoot, resolved);
if (relativePath === '..' || relativePath.startsWith(`..${sep}`) || isAbsolute(relativePath)) return [];
if (this.isBlocked(resolved)) {
return [];
}
try {
const rootStat = await lstat(resolved); // codeql[js/path-injection]
const rootStat = await lstat(resolved);
if (rootStat.isSymbolicLink()) return [];
const entries = await readdir(resolved, { withFileTypes: true }); // codeql[js/path-injection]
const entries = await readdir(resolved, { withFileTypes: true });
const dirs: DirectoryEntry[] = [];
for (const entry of entries) {
if (!entry.isDirectory()) continue;
if (!entry.isDirectory() || entry.isSymbolicLink()) continue;
if (entry.name.startsWith('.')) continue;
const full = join(resolved, entry.name);
try {
const s = await lstat(full); // codeql[js/path-injection]
if (s.isDirectory() && !s.isSymbolicLink()) dirs.push({ name: entry.name, path: full });
} catch {
// skip inaccessible entries
}
dirs.push({ name: entry.name, path: full });
}
return dirs.sort((a, b) => a.name.localeCompare(b.name));
} catch {
@@ -56,7 +55,13 @@ export class PathService {
async createDirectory(rawParent: string, rawName: string): Promise<CreateFolderResult> {
const name = this.assertSafeName(rawName);
const parent = await this.pathPolicy.resolveBrowsePath(rawParent);
const browseRoot = resolve(this.pathPolicy.getBrowseRoot());
const parent = resolve(await this.pathPolicy.resolveBrowsePath(rawParent));
const relativeParent = relative(browseRoot, parent);
if (relativeParent === '..' || relativeParent.startsWith(`..${sep}`) || isAbsolute(relativeParent)) {
throw new ForbiddenException('Cannot create a folder outside the configured library browse root');
}
if (this.isBlocked(parent)) {
throw new ForbiddenException('Cannot create a folder in this location');
}
@@ -64,21 +69,23 @@ export class PathService {
const startedAt = Date.now();
this.logger.log(`[path.create_folder] [start] parentPath="${sanitizeLogValue(parent)}" name="${sanitizeLogValue(name)}" - create folder started`);
try {
const parentStat = await lstat(parent); // codeql[js/path-injection]
const parentStat = await lstat(parent);
if (parentStat.isSymbolicLink()) throw new ForbiddenException('Cannot create a folder under a symbolic link');
if (!parentStat.isDirectory()) throw new BadRequestException('Parent path is not a directory');
const target = join(parent, name);
const prefix = parent.endsWith(sep) ? parent : parent + sep;
const resolvedTarget = resolve(target);
if (!resolvedTarget.startsWith(prefix)) {
const relativeTarget = relative(parent, resolvedTarget);
if (relativeTarget === '..' || relativeTarget.startsWith(`..${sep}`) || isAbsolute(relativeTarget)) {
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`);
return { name, path: target };
await mkdir(resolvedTarget);
this.logger.log(
`[path.create_folder] [end] path="${sanitizeLogValue(resolvedTarget)}" durationMs=${Date.now() - startedAt} - create folder completed`,
);
return { name, path: resolvedTarget };
} catch (error) {
this.logger.warn(
`[path.create_folder] [fail] parentPath="${sanitizeLogValue(parent)}" name="${sanitizeLogValue(name)}" durationMs=${Date.now() - startedAt} errorClass=${getErrorClass(error)} error="${sanitizeLogValue(getErrorMessage(error))}" - create folder failed`,
@@ -93,15 +100,16 @@ export class PathService {
private assertSafeName(rawName: string): string {
const name = (rawName ?? '').trim();
const safeName = basename(name);
if (!name) throw new BadRequestException('Folder name is required');
if (name.includes('/') || name.includes('\\') || name.includes(sep)) {
if (safeName !== name || safeName.includes('/') || safeName.includes('\\') || safeName.includes(sep)) {
throw new BadRequestException('Folder name cannot contain path separators');
}
if (name === '.' || name === '..' || name.startsWith('.')) {
if (safeName === '.' || safeName === '..' || safeName.startsWith('.')) {
throw new BadRequestException('Folder name cannot start with a dot');
}
if (name.includes('\0')) throw new BadRequestException('Folder name contains invalid characters');
return name;
if (safeName.includes('\0')) throw new BadRequestException('Folder name contains invalid characters');
return safeName;
}
private mapCreateError(error: unknown): HttpException {
@@ -79,6 +79,10 @@ describe('parseSpanSelector', () => {
it('round-trips through spanSelectorFromId', () => {
expect(parseSpanSelector(spanSelectorFromId('kobo.12.34'))).toBe('kobo.12.34');
});
it('escapes backslashes before building a selector', () => {
expect(spanSelectorFromId('kobo\\.12.34')).toBe('span#kobo\\\\\\.12\\.34');
});
});
describe('buildKoboSpanIndex', () => {
@@ -64,7 +64,7 @@ export function parseSpanSelector(path: string): string | null {
}
export function spanSelectorFromId(id: string): string {
return `span#${id.replace(/\./g, '\\.')}`;
return `span#${id.replace(/\\/g, '\\\\').replace(/\./g, '\\.')}`;
}
function isKoboSpanElement(node: CfiNode): boolean {
@@ -35,7 +35,13 @@ describe('lintHighlights', () => {
it('warns when an explicit image uses a non-allowlisted host', () => {
const body = ['## Highlights', '- **Thing** - body.', '![](https://imgur.com/x.png)'].join('\n');
const r = lintHighlights(body);
expect(r.warnings.some((w) => w.includes('dropped') && w.includes('imgur.com'))).toBe(true);
expect(
r.warnings.some(
(w) =>
w ===
'Media will be dropped (host not allowed): https://imgur.com/x.png. Use a GitHub user-attachments or githubusercontent URL (drag-drop into the release).',
),
).toBe(true);
});
it('does not warn for an allowlisted image', () => {
@@ -159,6 +159,13 @@ describe('parseHighlights', () => {
expect(parseHighlights(body)).toEqual([]);
});
it('removes malformed nested tag-like text without creating executable markup', () => {
const result = parseHighlights('## Highlights\n- **Safe** - before <scr<script>ipt>alert(1)</scr</script>ipt> after')[0];
expect(result.title).toBe('Safe');
expect(result.body).not.toContain('<script');
});
it('stops at the next heading but NOT at a horizontal rule', () => {
const body = ['## Highlights', '- **One** - first.', '', '---', '', '- **Two** - second.', '', '### Features', '- not a highlight'].join('\n');
expect(parseHighlights(body).map((h) => h.title)).toEqual(['One', 'Two']);
@@ -1,5 +1,6 @@
import type { ReleaseHighlight, ReleaseMedia } from '@bookorbit/types';
import { htmlToPlainText } from '../../common/utils/html-to-text.utils';
import { MAX_HIGHLIGHTS_PER_RELEASE, MAX_MEDIA_PER_HIGHLIGHT } from './release-notes.constants';
const HEADING_RE = /^#{1,6}\s+/;
@@ -18,14 +19,29 @@ const MEDIA_LINK_RE = /!?\[[^\]]*\]\(([^)\s]+)(?:\s+"[^"]*")?\)/g;
const IMAGE_MD_RE = /!\[[^\]]*\]\(([^)\s]+)(?:\s+"[^"]*")?\)/g;
// GitHub's web editor inserts dropped images as `<img ... src="...">` (and video sources similarly).
const HTML_MEDIA_SRC_RE = /<(?:img|video|source)\b[^>]*?\bsrc=["']([^"']+)["']/gi;
const HTML_TAG_RE = /<[^>]+>/g;
const BARE_URL_RE = /https?:\/\/[^\s)"'<>\]]+/g;
const VIDEO_EXT_RE = /\.(mp4|webm|mov|m4v|ogv|ogg)(?:[?#].*)?$/i;
const LEADING_SEP_RE = /^\s*[-:–—]\s*/;
/** Strip HTML comments but preserve standalone `<!-- icon: X -->` markers so the parser can read the icon. */
function stripHtmlComments(text: string): string {
return text.replace(/<!--[\s\S]*?-->/g, (m) => (ICON_COMMENT_ONLY_RE.test(m.trim()) ? m : ''));
let result = '';
let cursor = 0;
while (cursor < text.length) {
const start = text.indexOf('<!--', cursor);
if (start < 0) return result + text.slice(cursor);
result += text.slice(cursor, start);
const end = text.indexOf('-->', start + 4);
if (end < 0) return result;
const comment = text.slice(start, end + 3);
if (ICON_COMMENT_ONLY_RE.test(comment.trim())) result += comment;
cursor = end + 3;
}
return result;
}
function isHighlightsHeading(line: string): boolean {
@@ -130,7 +146,7 @@ export function parseHighlightEntries(body: string | null | undefined): ParsedHi
const iconComment = combined.match(ICON_COMMENT_RE);
if (iconComment) icon = iconComment[1];
let text = entry.text.replace(HTML_TAG_RE, '').replace(MEDIA_LINK_RE, '').replace(BARE_URL_RE, '').trim();
let text = htmlToPlainText(entry.text).replace(MEDIA_LINK_RE, '').replace(BARE_URL_RE, '').trim();
const tokenMatch = text.match(ICON_TOKEN_RE);
if (tokenMatch) {
@@ -88,7 +88,7 @@ describe('ReleaseNotesService.getSince', () => {
} as unknown as ConfigService;
const appSettings = { isUpdateCheckEnabled: vi.fn().mockResolvedValue(true) } as unknown as AppSettingsService;
const fetchMock = vi.fn((url: string) =>
url.includes('api.github.com')
new URL(url).hostname === 'api.github.com'
? Promise.resolve({ ok: true, json: () => Promise.resolve(releases) })
: Promise.resolve({ headers: { get: () => 'video/mp4' } }),
);
@@ -97,7 +97,7 @@ describe('ReleaseNotesService.getSince', () => {
const highlights = (await service.getSince('v1.0.0')).releases[0].highlights;
expect(highlights[0].media).toEqual([{ url: sharedUrl, type: 'video' }]);
expect(highlights[1].media).toEqual([{ url: sharedUrl, type: 'video' }]);
const probeCalls = fetchMock.mock.calls.filter(([u]) => !String(u).includes('api.github.com'));
const probeCalls = fetchMock.mock.calls.filter(([u]) => new URL(String(u)).hostname !== 'api.github.com');
expect(probeCalls).toHaveLength(1);
});
@@ -8,6 +8,7 @@ import {
replacePlaceholders,
resolveDownloadFilename,
resolveUploadPath,
sanitizePathSegment,
validatePattern,
} from '@bookorbit/types';
@@ -33,6 +34,12 @@ const DECIMAL_INDEX: Record<string, string> = {
seriesIndex: '01.5',
};
describe('sanitizePathSegment', () => {
it('removes a large trailing run of dots and spaces in linear time', () => {
expect(sanitizePathSegment(`book${'. '.repeat(100_000)}`)).toBe('book');
});
});
// ── applyModifier ─────────────────────────────────────────────────────────────
describe('applyModifier', () => {