fix(authors): harden module behavior and safety checks

This commit is contained in:
neon
2026-03-31 03:11:22 -06:00
parent ec44071a91
commit 7952e32800
12 changed files with 314 additions and 23 deletions
@@ -1,7 +1,14 @@
import { AuthorEnrichmentGateway } from './author-enrichment.gateway';
function makeGateway() {
return new AuthorEnrichmentGateway({ verify: vi.fn() } as any, { validateUser: vi.fn() } as any, { getStatusSummary: vi.fn() } as any);
return new AuthorEnrichmentGateway(
{ verify: vi.fn() } as any,
{ validateUser: vi.fn() } as any,
{ getStatusSummary: vi.fn() } as any,
{ isPaused: vi.fn() } as any,
{ getSnapshot: vi.fn() } as any,
{ get: vi.fn().mockReturnValue('http://localhost:5173') } as any,
);
}
describe('AuthorEnrichmentGateway', () => {
@@ -1,29 +1,43 @@
import { Logger } from '@nestjs/common';
import { ConfigService } from '@nestjs/config';
import { JwtService } from '@nestjs/jwt';
import { OnGatewayConnection, OnGatewayDisconnect, WebSocketGateway, WebSocketServer } from '@nestjs/websockets';
import { OnGatewayConnection, OnGatewayDisconnect, OnGatewayInit, WebSocketGateway, WebSocketServer } from '@nestjs/websockets';
import { Permission, type AuthorEnrichmentStatusEvent } from '@projectx/types';
import { Server, Socket } from 'socket.io';
import type { RequestUser } from '../../common/types/request-user';
import { AuthService } from '../auth/auth.service';
import { AppSettingsService } from '../app-settings/app-settings.service';
import { AuthorEnrichmentRepository } from './author-enrichment.repository';
import { AuthorEnrichmentSessionService } from './author-enrichment-session.service';
import { AuthorEnrichmentConfigService } from './author-enrichment-config.service';
export const AUTHOR_ENRICHMENT_STATUS_EVENT = 'author-enrichment:status';
@WebSocketGateway({ namespace: '/authors-enrichment', cors: { origin: process.env.CLIENT_URL ?? 'http://localhost:5173' } })
export class AuthorEnrichmentGateway implements OnGatewayConnection, OnGatewayDisconnect {
@WebSocketGateway({ namespace: '/authors-enrichment', cors: { credentials: true } })
export class AuthorEnrichmentGateway implements OnGatewayInit, OnGatewayConnection, OnGatewayDisconnect {
@WebSocketServer() server: Server;
private readonly logger = new Logger(AuthorEnrichmentGateway.name);
private readonly clientOrigin: string;
constructor(
private readonly jwtService: JwtService,
private readonly authService: AuthService,
private readonly queueRepo: AuthorEnrichmentRepository,
private readonly appSettings: AppSettingsService,
private readonly enrichmentConfig: AuthorEnrichmentConfigService,
private readonly session: AuthorEnrichmentSessionService,
) {}
config: ConfigService,
) {
this.clientOrigin = config.get<string>('app.appUrl') ?? 'http://localhost:5173';
}
afterInit(server: Server): void {
if (!server.engine?.opts) return;
server.engine.opts.cors = {
...(server.engine.opts.cors ?? {}),
origin: this.clientOrigin,
credentials: true,
};
}
async handleConnection(client: Socket): Promise<void> {
try {
@@ -37,7 +51,7 @@ export class AuthorEnrichmentGateway implements OnGatewayConnection, OnGatewayDi
this.assertCanViewStatus(user);
(client.data as Record<string, unknown>).user = user;
this.logger.debug(`WS connected: user=${user.id} socket=${client.id}`);
const [summary, paused] = await Promise.all([this.queueRepo.getStatusSummary(), this.appSettings.isAuthorEnrichmentPaused()]);
const [summary, paused] = await Promise.all([this.queueRepo.getStatusSummary(), this.enrichmentConfig.isPaused()]);
client.emit(AUTHOR_ENRICHMENT_STATUS_EVENT, { ...summary, paused, ...this.session.getSnapshot() });
} catch (err) {
this.logger.warn(`WS rejected: ${(err as Error).message} socket=${client.id}`);
@@ -12,8 +12,6 @@ type Db = NodePgDatabase<typeof schema>;
export const AUTHOR_ENRICHMENT_ACTIVE_STATUSES = ['queued', 'rate_limited'] as const;
export type AuthorEnrichmentActiveStatus = (typeof AUTHOR_ENRICHMENT_ACTIVE_STATUSES)[number];
export type AuthorEnrichmentQueueRow = typeof authorEnrichmentQueue.$inferSelect;
@Injectable()
@@ -0,0 +1,77 @@
import { mkdtemp, mkdir, readFile, rm, writeFile } from 'fs/promises';
import { tmpdir } from 'os';
import { join } from 'path';
import { lookup } from 'dns/promises';
import { AuthorImageStorageService } from './author-image-storage.service';
vi.mock('../metadata/lib/cover', () => ({
generateThumbnail: vi.fn(() => Promise.resolve(Buffer.from('thumbnail-bytes'))),
imageExt: vi.fn(() => 'jpg'),
}));
vi.mock('dns/promises', () => ({
lookup: vi.fn(),
}));
describe('AuthorImageStorageService', () => {
let booksPath: string;
let service: AuthorImageStorageService;
beforeEach(async () => {
vi.resetAllMocks();
booksPath = await mkdtemp(join(tmpdir(), 'authors-image-storage-'));
service = new AuthorImageStorageService({
get: vi.fn().mockReturnValue(booksPath),
} as never);
});
afterEach(async () => {
await rm(booksPath, { recursive: true, force: true });
});
it('rejects localhost URLs before fetch', async () => {
global.fetch = vi.fn();
await expect(service.saveFromUrl(1, 'http://localhost/image.jpg')).resolves.toBe(false);
expect(global.fetch).not.toHaveBeenCalled();
});
it('rejects URLs that resolve to private IPs', async () => {
vi.mocked(lookup).mockResolvedValue([{ address: '127.0.0.1', family: 4 }] as never);
global.fetch = vi.fn();
await expect(service.saveFromUrl(1, 'https://example.test/image.jpg')).resolves.toBe(false);
expect(global.fetch).not.toHaveBeenCalled();
});
it('rejects direct private IP URLs', async () => {
global.fetch = vi.fn();
await expect(service.saveFromUrl(1, 'http://10.0.0.4/image.jpg')).resolves.toBe(false);
expect(global.fetch).not.toHaveBeenCalled();
});
it('saves image and thumbnail for public hosts and removes old photo files', async () => {
vi.mocked(lookup).mockResolvedValue([{ address: '93.184.216.34', family: 4 }] as never);
global.fetch = vi.fn().mockResolvedValue({
ok: true,
headers: { get: vi.fn().mockReturnValue('image/jpeg') },
body: (async function* () {
await Promise.resolve();
yield Buffer.from('image-bytes');
})(),
} as never);
const authorDir = join(booksPath, 'authors', '42');
await mkdir(authorDir, { recursive: true });
await writeFile(join(authorDir, 'photo.png'), Buffer.from('old-photo'));
await expect(service.saveFromUrl(42, 'https://cdn.example.com/author.jpg')).resolves.toBe(true);
await expect(readFile(join(authorDir, 'photo.jpg'))).resolves.toEqual(Buffer.from('image-bytes'));
await expect(readFile(join(authorDir, 'thumbnail.jpg'))).resolves.toEqual(Buffer.from('thumbnail-bytes'));
await expect(readFile(join(authorDir, 'photo.png'))).rejects.toThrow();
});
});
@@ -1,7 +1,9 @@
import { Injectable, Logger } from '@nestjs/common';
import { ConfigService } from '@nestjs/config';
import { lookup } from 'dns/promises';
import { access, mkdir, readdir, stat, unlink, writeFile } from 'fs/promises';
import { constants as fsConstants } from 'fs';
import { isIP } from 'net';
import { join } from 'path';
import { generateThumbnail, imageExt } from '../metadata/lib/cover';
@@ -19,7 +21,7 @@ export class AuthorImageStorageService {
}
async saveFromUrl(authorId: number, rawUrl: string): Promise<boolean> {
const url = this.normalizeUrl(rawUrl);
const url = await this.normalizeUrl(rawUrl);
if (!url) return false;
try {
@@ -31,7 +33,10 @@ export class AuthorImageStorageService {
const existing = await readdir(dir).catch(() => [] as string[]);
for (const file of existing.filter((entry) => entry.startsWith('photo.'))) {
await unlink(join(dir, file)).catch(() => {});
await unlink(join(dir, file)).catch((error: unknown) => {
const message = error instanceof Error ? error.message : String(error);
this.logger.warn(`author image cleanup failed for authorId=${authorId} file=${file}: ${message}`);
});
}
const ext = imageExt(bytes);
@@ -96,7 +101,7 @@ export class AuthorImageStorageService {
}
}
private normalizeUrl(rawUrl: string): string | null {
private async normalizeUrl(rawUrl: string): Promise<string | null> {
const trimmed = rawUrl.trim();
if (!trimmed) return null;
@@ -113,9 +118,33 @@ export class AuthorImageStorageService {
return null;
}
if (!(await this.isSafeRemoteHost(parsed.hostname))) {
return null;
}
return parsed.toString();
}
private async isSafeRemoteHost(hostname: string): Promise<boolean> {
const normalizedHost = hostname.trim().toLowerCase();
if (!normalizedHost) return false;
if (normalizedHost === 'localhost' || normalizedHost.endsWith('.localhost') || normalizedHost.endsWith('.local')) {
return false;
}
if (isIP(normalizedHost) > 0) {
return !isPrivateOrLocalAddress(normalizedHost);
}
try {
const resolved = await lookup(normalizedHost, { all: true, verbatim: true });
if (resolved.length === 0) return false;
return resolved.every((entry) => !isPrivateOrLocalAddress(entry.address));
} catch {
return false;
}
}
private async fetchImageFromUrl(url: string): Promise<Buffer | null> {
const controller = new AbortController();
const timeout = setTimeout(() => controller.abort(), FETCH_TIMEOUT_MS);
@@ -149,3 +178,45 @@ export class AuthorImageStorageService {
}
}
}
function isPrivateOrLocalAddress(address: string): boolean {
const normalized = address.toLowerCase();
const mappedV4Prefix = '::ffff:';
const maybeV4 = normalized.startsWith(mappedV4Prefix) ? normalized.slice(mappedV4Prefix.length) : normalized;
const family = isIP(maybeV4);
if (family === 4) {
return isPrivateOrLocalV4(maybeV4);
}
if (family === 6) {
return (
maybeV4 === '::1' ||
maybeV4 === '::' ||
maybeV4.startsWith('fc') ||
maybeV4.startsWith('fd') ||
maybeV4.startsWith('fe8') ||
maybeV4.startsWith('fe9') ||
maybeV4.startsWith('fea') ||
maybeV4.startsWith('feb')
);
}
return true;
}
function isPrivateOrLocalV4(address: string): boolean {
const octets = address.split('.').map((part) => Number.parseInt(part, 10));
if (octets.length !== 4 || octets.some((value) => !Number.isFinite(value) || value < 0 || value > 255)) {
return true;
}
const [a, b] = octets;
if (a === 10 || a === 127) return true;
if (a === 172 && b >= 16 && b <= 31) return true;
if (a === 192 && b === 168) return true;
if (a === 169 && b === 254) return true;
if (a === 0) return true;
return false;
}
@@ -27,8 +27,9 @@ import { CurrentUser } from '../../common/decorators/current-user.decorator';
import { Auditable } from '../../common/decorators/auditable.decorator';
import { RequirePermission } from '../../common/decorators/require-permission.decorator';
import type { RequestUser } from '../../common/types/request-user';
import type { AuthorAutoEnrichmentConfig, AuthorEnrichmentConditions, AuthorMetadataCandidate } from '@projectx/types';
import type { AuthorMetadataCandidate } from '@projectx/types';
import { AuthorEnrichmentConfigService } from './author-enrichment-config.service';
import { AuthorAutoEnrichmentConfigDto } from './dto/author-auto-enrichment-config.dto';
import { BulkAuthorIdsDto } from './dto/bulk-author-ids.dto';
import { DeleteAuthorsDto } from './dto/delete-authors.dto';
import { ListAuthorBooksDto } from './dto/list-author-books.dto';
@@ -38,6 +39,7 @@ import { ListAuthorsDto } from './dto/list-authors.dto';
import { ListDuplicateSuggestionsDto } from './dto/list-duplicate-suggestions.dto';
import { LookupAuthorMetadataDto } from './dto/lookup-author-metadata.dto';
import { MergeAuthorsDto } from './dto/merge-authors.dto';
import { PreviewAuthorEnrichmentCountDto } from './dto/preview-author-enrichment-count.dto';
import { UpdateAuthorDto } from './dto/update-author.dto';
import { AuthorsService } from './authors.service';
import { AuthorEnrichmentOrchestratorService } from './author-enrichment-orchestrator.service';
@@ -112,14 +114,14 @@ export class AuthorsController {
@HttpCode(HttpStatus.OK)
@RequirePermission(Permission.ManageMetadataConfig)
@Auditable({ action: AuditAction.AuthorEnrichmentConfigUpdate, description: 'Updated author enrichment configuration' })
async setEnrichmentConfig(@Body() config: AuthorAutoEnrichmentConfig) {
async setEnrichmentConfig(@Body() config: AuthorAutoEnrichmentConfigDto) {
await this.enrichmentConfig.setConfig(config);
return this.enrichmentConfig.getConfig();
}
@Post('enrichment/preview-count')
@RequirePermission(Permission.ManageMetadataConfig)
async previewCount(@Body() body: { conditions: AuthorEnrichmentConditions }) {
async previewCount(@Body() body: PreviewAuthorEnrichmentCountDto) {
const count = await this.queueRepo.countEligibleLinkedAuthors(body.conditions);
return { count };
}
@@ -0,0 +1,30 @@
import { AuthorAutoEnrichmentWriteMode } from '@projectx/types';
import { Type } from 'class-transformer';
import { IsBoolean, IsDefined, IsEnum, ValidateNested } from 'class-validator';
export class AuthorEnrichmentConditionsDto {
@IsBoolean()
neverEnriched!: boolean;
@IsBoolean()
missingBio!: boolean;
@IsBoolean()
missingPhoto!: boolean;
}
export class AuthorAutoEnrichmentConfigDto {
@IsBoolean()
enabled!: boolean;
@IsBoolean()
triggerOnImport!: boolean;
@IsEnum(AuthorAutoEnrichmentWriteMode)
writeMode!: AuthorAutoEnrichmentWriteMode;
@IsDefined()
@ValidateNested()
@Type(() => AuthorEnrichmentConditionsDto)
conditions!: AuthorEnrichmentConditionsDto;
}
@@ -0,0 +1,48 @@
import 'reflect-metadata';
import { plainToInstance } from 'class-transformer';
import { validate } from 'class-validator';
import { AuthorAutoEnrichmentWriteMode } from '@projectx/types';
import { AuthorAutoEnrichmentConfigDto } from './author-auto-enrichment-config.dto';
import { PreviewAuthorEnrichmentCountDto } from './preview-author-enrichment-count.dto';
describe('author enrichment DTO validation', () => {
it('accepts valid enrichment config payload', async () => {
const dto = plainToInstance(AuthorAutoEnrichmentConfigDto, {
enabled: true,
triggerOnImport: false,
writeMode: AuthorAutoEnrichmentWriteMode.MISSING_ONLY,
conditions: {
neverEnriched: true,
missingBio: false,
missingPhoto: true,
},
});
await expect(validate(dto)).resolves.toHaveLength(0);
});
it('rejects invalid writeMode and non-boolean conditions', async () => {
const dto = plainToInstance(AuthorAutoEnrichmentConfigDto, {
enabled: true,
triggerOnImport: true,
writeMode: 'bad_mode',
conditions: {
neverEnriched: 'true',
missingBio: false,
missingPhoto: true,
},
});
const errors = await validate(dto);
expect(errors.length).toBeGreaterThan(0);
});
it('rejects preview-count payload when conditions are missing', async () => {
const dto = plainToInstance(PreviewAuthorEnrichmentCountDto, {});
const errors = await validate(dto);
expect(errors.length).toBeGreaterThan(0);
});
});
@@ -0,0 +1,20 @@
import { Type } from 'class-transformer';
import { IsBoolean, IsDefined, ValidateNested } from 'class-validator';
export class PreviewAuthorEnrichmentConditionsDto {
@IsBoolean()
neverEnriched!: boolean;
@IsBoolean()
missingBio!: boolean;
@IsBoolean()
missingPhoto!: boolean;
}
export class PreviewAuthorEnrichmentCountDto {
@IsDefined()
@ValidateNested()
@Type(() => PreviewAuthorEnrichmentConditionsDto)
conditions!: PreviewAuthorEnrichmentConditionsDto;
}
@@ -13,5 +13,6 @@ export class UpdateAuthorDto {
@IsOptional()
@IsString()
@MaxLength(10_000)
description?: string | null;
}
@@ -1,3 +1,5 @@
import { firstValueFrom, toArray } from 'rxjs';
import { AuthorMetadataProviderError } from './providers/author-metadata-provider';
import { AuthorMetadataFetchService } from './author-metadata-fetch.service';
@@ -68,4 +70,30 @@ describe('AuthorMetadataFetchService', () => {
},
});
});
it('stream enforces global result limit across providers', async () => {
registry.select.mockReturnValue([
{
key: 'audnexus',
label: 'Audnexus',
identifiable: false,
search: vi.fn().mockResolvedValue([
{ provider: 'audnexus', providerId: 'A1', name: 'Author One' },
{ provider: 'audnexus', providerId: 'A2', name: 'Author Two' },
]),
},
{
key: 'other',
label: 'Other',
identifiable: false,
search: vi.fn().mockResolvedValue([
{ provider: 'other', providerId: 'B1', name: 'Author Three' },
{ provider: 'other', providerId: 'B2', name: 'Author Four' },
]),
},
]);
const emitted = await firstValueFrom(service.stream({ name: 'author', limit: 2 }).pipe(toArray()));
expect(emitted).toHaveLength(2);
});
});
@@ -1,6 +1,6 @@
import { Injectable } from '@nestjs/common';
import { AuthorMetadataCandidate, AuthorMetadataProviderInfo, AuthorMetadataProviderKey } from '@projectx/types';
import { from, merge, Observable, switchMap } from 'rxjs';
import { from, merge, Observable, switchMap, take } from 'rxjs';
import { AuthorMetadataProviderRegistry } from './provider-registry';
import { AuthorMetadataProviderError, AuthorMetadataSearchParams, isIdentifiableAuthorProvider } from './providers/author-metadata-provider';
@@ -73,12 +73,7 @@ export class AuthorMetadataFetchService {
),
).pipe(switchMap((candidates) => from(candidates.slice(0, limit)))),
),
);
}
async quickSearch(params: AuthorMetadataSearchParams, options?: SearchOptions): Promise<AuthorMetadataCandidate | null> {
const detailed = await this.quickSearchDetailed(params, options);
return detailed.candidate;
).pipe(take(limit));
}
async quickSearchDetailed(params: AuthorMetadataSearchParams, options?: SearchOptions): Promise<AuthorQuickSearchResult> {