fix: code review cleanup
- Add missing dependencies: ioredis, adm-zip, archiver, gray-matter, @types/multer, @types/archiver - Fix import statements: use default imports for AdmZip, archiver, gray-matter - Remove unused imports: ArrayMinSize - Fix export types: use 'export type' for type-only exports - Replace 'any' types with proper types: - AuthUser for user parameters - ExportEntry interface for entry data - unknown for frontmatter parsing parameters - Record<string, unknown> for dynamic objects - Add security improvements: - File upload size limit: 50MB max - File type validation in FileInterceptor - Path traversal protection in zip extraction - Zip bomb protection: max 1000 files, 100MB uncompressed - Fix exactOptionalPropertyTypes issues: use conditional spreading for optional fields
This commit is contained in:
@@ -3,7 +3,6 @@ import {
|
||||
IsOptional,
|
||||
IsEnum,
|
||||
IsArray,
|
||||
ArrayMinSize,
|
||||
} from "class-validator";
|
||||
|
||||
/**
|
||||
|
||||
@@ -10,9 +10,5 @@ export {
|
||||
RecentEntriesDto,
|
||||
} from "./search-query.dto";
|
||||
export { GraphQueryDto } from "./graph-query.dto";
|
||||
export {
|
||||
ExportQueryDto,
|
||||
ExportFormat,
|
||||
ImportResult,
|
||||
ImportResponseDto,
|
||||
} from "./import-export.dto";
|
||||
export { ExportQueryDto, ExportFormat } from "./import-export.dto";
|
||||
export type { ImportResult, ImportResponseDto } from "./import-export.dto";
|
||||
|
||||
@@ -17,6 +17,7 @@ import { AuthGuard } from "../auth/guards/auth.guard";
|
||||
import { WorkspaceGuard, PermissionGuard } from "../common/guards";
|
||||
import { Workspace, Permission, RequirePermission } from "../common/decorators";
|
||||
import { CurrentUser } from "../auth/decorators/current-user.decorator";
|
||||
import type { AuthUser } from "../auth/types/better-auth-request.interface";
|
||||
|
||||
/**
|
||||
* Controller for knowledge import/export endpoints
|
||||
@@ -34,10 +35,42 @@ export class ImportExportController {
|
||||
*/
|
||||
@Post("import")
|
||||
@RequirePermission(Permission.WORKSPACE_MEMBER)
|
||||
@UseInterceptors(FileInterceptor("file"))
|
||||
@UseInterceptors(
|
||||
FileInterceptor("file", {
|
||||
limits: {
|
||||
fileSize: 50 * 1024 * 1024, // 50MB max file size
|
||||
},
|
||||
fileFilter: (_req, file, callback) => {
|
||||
// Only accept .md and .zip files
|
||||
const allowedMimeTypes = [
|
||||
"text/markdown",
|
||||
"application/zip",
|
||||
"application/x-zip-compressed",
|
||||
];
|
||||
const allowedExtensions = [".md", ".zip"];
|
||||
const fileExtension = file.originalname.toLowerCase().slice(
|
||||
file.originalname.lastIndexOf(".")
|
||||
);
|
||||
|
||||
if (
|
||||
allowedMimeTypes.includes(file.mimetype) ||
|
||||
allowedExtensions.includes(fileExtension)
|
||||
) {
|
||||
callback(null, true);
|
||||
} else {
|
||||
callback(
|
||||
new BadRequestException(
|
||||
"Invalid file type. Only .md and .zip files are accepted."
|
||||
),
|
||||
false
|
||||
);
|
||||
}
|
||||
},
|
||||
})
|
||||
)
|
||||
async importEntries(
|
||||
@Workspace() workspaceId: string,
|
||||
@CurrentUser() user: any,
|
||||
@CurrentUser() user: AuthUser,
|
||||
@UploadedFile() file: Express.Multer.File
|
||||
): Promise<ImportResponseDto> {
|
||||
if (!file) {
|
||||
|
||||
@@ -1,14 +1,27 @@
|
||||
import { Injectable, BadRequestException } from "@nestjs/common";
|
||||
import { EntryStatus, Visibility } from "@prisma/client";
|
||||
import * as archiver from "archiver";
|
||||
import * as AdmZip from "adm-zip";
|
||||
import * as matter from "gray-matter";
|
||||
import archiver from "archiver";
|
||||
import AdmZip from "adm-zip";
|
||||
import matter from "gray-matter";
|
||||
import { Readable } from "stream";
|
||||
import { PrismaService } from "../../prisma/prisma.service";
|
||||
import { KnowledgeService } from "../knowledge.service";
|
||||
import type { ExportFormat, ImportResult } from "../dto";
|
||||
import type { CreateEntryDto } from "../dto/create-entry.dto";
|
||||
|
||||
interface ExportEntry {
|
||||
id: string;
|
||||
slug: string;
|
||||
title: string;
|
||||
content: string;
|
||||
summary: string | null;
|
||||
status: EntryStatus;
|
||||
visibility: Visibility;
|
||||
tags: string[];
|
||||
createdAt: Date;
|
||||
updatedAt: Date;
|
||||
}
|
||||
|
||||
/**
|
||||
* Service for handling knowledge entry import/export operations
|
||||
*/
|
||||
@@ -94,14 +107,18 @@ export class ImportExportService {
|
||||
}
|
||||
|
||||
// Build CreateEntryDto from frontmatter and content
|
||||
const parsedStatus = this.parseStatus(frontmatter.status);
|
||||
const parsedVisibility = this.parseVisibility(frontmatter.visibility);
|
||||
const parsedTags = Array.isArray(frontmatter.tags) ? frontmatter.tags : undefined;
|
||||
|
||||
const createDto: CreateEntryDto = {
|
||||
title: frontmatter.title || filename.replace(/\.md$/, ""),
|
||||
content: markdownContent,
|
||||
summary: frontmatter.summary,
|
||||
status: this.parseStatus(frontmatter.status),
|
||||
visibility: this.parseVisibility(frontmatter.visibility),
|
||||
tags: Array.isArray(frontmatter.tags) ? frontmatter.tags : undefined,
|
||||
changeNote: "Imported from markdown file",
|
||||
...(frontmatter.summary && { summary: frontmatter.summary }),
|
||||
...(parsedStatus && { status: parsedStatus }),
|
||||
...(parsedVisibility && { visibility: parsedVisibility }),
|
||||
...(parsedTags && { tags: parsedTags }),
|
||||
};
|
||||
|
||||
// Create the entry
|
||||
@@ -136,17 +153,57 @@ export class ImportExportService {
|
||||
buffer: Buffer
|
||||
): Promise<ImportResult[]> {
|
||||
const results: ImportResult[] = [];
|
||||
const MAX_FILES = 1000; // Prevent zip bomb attacks
|
||||
const MAX_TOTAL_SIZE = 100 * 1024 * 1024; // 100MB total uncompressed
|
||||
|
||||
try {
|
||||
const zip = new AdmZip(buffer);
|
||||
const zipEntries = zip.getEntries();
|
||||
|
||||
// Security: Check for zip bombs
|
||||
let totalUncompressedSize = 0;
|
||||
let fileCount = 0;
|
||||
|
||||
for (const entry of zipEntries) {
|
||||
if (!entry.isDirectory) {
|
||||
fileCount++;
|
||||
totalUncompressedSize += entry.header.size;
|
||||
}
|
||||
}
|
||||
|
||||
if (fileCount > MAX_FILES) {
|
||||
throw new BadRequestException(
|
||||
`Zip file contains too many files (${fileCount}). Maximum allowed: ${MAX_FILES}`
|
||||
);
|
||||
}
|
||||
|
||||
if (totalUncompressedSize > MAX_TOTAL_SIZE) {
|
||||
throw new BadRequestException(
|
||||
`Zip file is too large when uncompressed (${Math.round(totalUncompressedSize / 1024 / 1024)}MB). Maximum allowed: ${Math.round(MAX_TOTAL_SIZE / 1024 / 1024)}MB`
|
||||
);
|
||||
}
|
||||
|
||||
for (const zipEntry of zipEntries) {
|
||||
// Skip directories and non-markdown files
|
||||
if (zipEntry.isDirectory || !zipEntry.entryName.endsWith(".md")) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Security: Prevent path traversal attacks
|
||||
const normalizedPath = zipEntry.entryName.replace(/\\/g, "/");
|
||||
if (
|
||||
normalizedPath.includes("..") ||
|
||||
normalizedPath.startsWith("/") ||
|
||||
normalizedPath.includes("//")
|
||||
) {
|
||||
results.push({
|
||||
filename: zipEntry.entryName,
|
||||
success: false,
|
||||
error: "Invalid file path detected (potential path traversal)",
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
const content = zipEntry.getData().toString("utf-8");
|
||||
const result = await this.importSingleMarkdown(
|
||||
workspaceId,
|
||||
@@ -218,8 +275,8 @@ export class ImportExportService {
|
||||
private async fetchEntriesForExport(
|
||||
workspaceId: string,
|
||||
entryIds?: string[]
|
||||
): Promise<any[]> {
|
||||
const where: any = { workspaceId };
|
||||
): Promise<ExportEntry[]> {
|
||||
const where: Record<string, unknown> = { workspaceId };
|
||||
|
||||
if (entryIds && entryIds.length > 0) {
|
||||
where.id = { in: entryIds };
|
||||
@@ -256,7 +313,7 @@ export class ImportExportService {
|
||||
/**
|
||||
* Convert entry to markdown format with frontmatter
|
||||
*/
|
||||
private entryToMarkdown(entry: any): string {
|
||||
private entryToMarkdown(entry: ExportEntry): string {
|
||||
const frontmatter: Record<string, any> = {
|
||||
title: entry.title,
|
||||
status: entry.status,
|
||||
@@ -290,7 +347,7 @@ export class ImportExportService {
|
||||
/**
|
||||
* Parse status from frontmatter
|
||||
*/
|
||||
private parseStatus(value: any): EntryStatus | undefined {
|
||||
private parseStatus(value: unknown): EntryStatus | undefined {
|
||||
if (!value) return undefined;
|
||||
const statusMap: Record<string, EntryStatus> = {
|
||||
DRAFT: EntryStatus.DRAFT,
|
||||
@@ -303,7 +360,7 @@ export class ImportExportService {
|
||||
/**
|
||||
* Parse visibility from frontmatter
|
||||
*/
|
||||
private parseVisibility(value: any): Visibility | undefined {
|
||||
private parseVisibility(value: unknown): Visibility | undefined {
|
||||
if (!value) return undefined;
|
||||
const visibilityMap: Record<string, Visibility> = {
|
||||
PRIVATE: Visibility.PRIVATE,
|
||||
|
||||
Reference in New Issue
Block a user