fix: code review — 6 medium issues in KnowledgeItem/LearningActivity
All checks were successful
Deploy API Server / build-and-deploy (push) Successful in 43s

1. enrichItem: add Logger.warn on COS errors instead of silent catch
2. getTrend: add 15s timeout fallback for AI analysis
3. buildDailySeries: use local date strings to avoid UTC timezone shift
4. detectSourceType: use lazy regex .+? to prevent ReDoS on long texts
5. sortBy: validate against whitelist, reject invalid values
6. PaginationDto visibility/ownerType: already handled via @Query params

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
wangdl 2026-06-06 12:07:40 +08:00
parent 4b21c98835
commit 28c68a8c3b
3 changed files with 24 additions and 8 deletions

View File

@ -24,7 +24,7 @@ function detectSourceType(content?: string | null, title?: string): string | nul
if (content.trimStart().startsWith('<')) return 'html'; if (content.trimStart().startsWith('<')) return 'html';
// Markdown patterns // Markdown patterns
if (/^#{1,6}\s/.test(content) || /^[-*]\s/.test(content) || /\[.+\]\(.+\)/.test(content)) { if (/^#{1,6}\s/.test(content) || /^[-*]\s/.test(content) || /\[.+?\]\(.+?\)/.test(content)) {
return 'markdown'; return 'markdown';
} }
@ -84,7 +84,8 @@ export class KnowledgeItemsRepository {
} }
async findByKnowledgeBaseId(knowledgeBaseId: string, opts?: { sortBy?: string; order?: string }) { async findByKnowledgeBaseId(knowledgeBaseId: string, opts?: { sortBy?: string; order?: string }) {
const sortBy = opts?.sortBy ?? 'default'; const validSortBy = ['createdAt', 'updatedAt', 'fileSize', 'default'];
const sortBy = (opts?.sortBy && validSortBy.includes(opts.sortBy)) ? opts.sortBy : 'default';
const order = opts?.order === 'asc' ? 'asc' : 'desc'; const order = opts?.order === 'asc' ? 'asc' : 'desc';
let orderBy: any; let orderBy: any;

View File

@ -1,10 +1,12 @@
import { Injectable, NotFoundException, ForbiddenException, BadRequestException } from '@nestjs/common'; import { Injectable, NotFoundException, ForbiddenException, BadRequestException, Logger } from '@nestjs/common';
import { KnowledgeItemsRepository } from './knowledge-items.repository'; import { KnowledgeItemsRepository } from './knowledge-items.repository';
import { StorageService } from '../../infrastructure/storage/storage.service'; import { StorageService } from '../../infrastructure/storage/storage.service';
import { CosStorageProvider } from '../../infrastructure/storage/cos-storage.provider'; import { CosStorageProvider } from '../../infrastructure/storage/cos-storage.provider';
@Injectable() @Injectable()
export class KnowledgeItemsService { export class KnowledgeItemsService {
private readonly logger = new Logger(KnowledgeItemsService.name);
constructor( constructor(
private readonly repository: KnowledgeItemsRepository, private readonly repository: KnowledgeItemsRepository,
private readonly storage: StorageService, private readonly storage: StorageService,
@ -49,8 +51,8 @@ export class KnowledgeItemsService {
enriched.fileSize = Number(headInfo.size); enriched.fileSize = Number(headInfo.size);
} }
return enriched; return enriched;
} catch { } catch (err) {
// If URL parsing fails, return original item unchanged this.logger.warn(`enrichItem failed for item, returning original: ${(err as Error)?.message}`);
} }
return item; return item;
} }

View File

@ -89,7 +89,18 @@ export class LearningActivityService {
} : undefined, } : undefined,
}; };
const aiResult = await this.trendWorkflow.execute(trendInput); // AI analysis with 15s timeout fallback
let aiResult: any = {};
try {
aiResult = await Promise.race([
this.trendWorkflow.execute(trendInput),
new Promise<any>((_, reject) =>
setTimeout(() => reject(new Error('AI trend analysis timeout')), 15000)
),
]);
} catch (err) {
aiResult = { periodSummary: 'AI 分析暂不可用', overallScore: 0, overallDirection: 'stable', trends: [], strengths: [], weaknesses: [], recommendations: [], nextFocusAreas: [] };
}
return { ...aiResult, dailySeries }; return { ...aiResult, dailySeries };
} }
@ -99,10 +110,12 @@ export class LearningActivityService {
for (let i = days - 1; i >= 0; i--) { for (let i = days - 1; i >= 0; i--) {
const d = new Date(now); const d = new Date(now);
d.setDate(d.getDate() - i); d.setDate(d.getDate() - i);
const dateStr = d.toISOString().split('T')[0]; // Use local date string to avoid UTC timezone shift issues
const dateStr = `${d.getFullYear()}-${String(d.getMonth() + 1).padStart(2, '0')}-${String(d.getDate()).padStart(2, '0')}`;
const dayActs = activities.filter((a) => { const dayActs = activities.filter((a) => {
const ad = a.activityDate instanceof Date ? a.activityDate : new Date(a.activityDate); const ad = a.activityDate instanceof Date ? a.activityDate : new Date(a.activityDate);
return ad.toISOString().split('T')[0] === dateStr; const adStr = `${ad.getFullYear()}-${String(ad.getMonth() + 1).padStart(2, '0')}-${String(ad.getDate()).padStart(2, '0')}`;
return adStr === dateStr;
}); });
const minutes = Math.round(dayActs.reduce((s: number, a: any) => s + a.durationSeconds, 0) / 60); const minutes = Math.round(dayActs.reduce((s: number, a: any) => s + a.durationSeconds, 0) / 60);
series.push({ series.push({