From 1ee9415edaa0cc02f0b8f4a15309a34536880678 Mon Sep 17 00:00:00 2001 From: sudacode Date: Thu, 16 Jul 2026 00:07:18 -0700 Subject: [PATCH] fix: harden AnkiConnect resolver and stats-server route validation - Warn and fall back for invalid primitive values across modern/legacy AnkiConnect config subtrees - Validate AniList ID as positive integer; add timeout to AniList search fetch - Reject non-safe or fractional session IDs; return 404 for malformed percent-encoded asset paths - Guard word-mode mining: require non-empty word and Yomitan bridge before starting media work - Include note ID in mined media filenames to prevent collisions - Silence throwing timing observers so they cannot abort mining execution - Trim whitespace from excludePos query param entries --- changes/split-resolvers.md | 5 + src/config/anki-connect-nplusone-migration.ts | 11 +- src/config/resolve/anki-connect.test.ts | 102 ++++++ src/config/resolve/anki-connect/initialize.ts | 12 - .../resolve/anki-connect/known-words.ts | 22 +- src/config/resolve/anki-connect/modern.ts | 300 ++++++++++++++++-- .../stats-server-mining-support.test.ts | 31 ++ .../services/__tests__/stats-server.test.ts | 173 +++++++++- .../stats-server/integration-routes.ts | 10 +- .../services/stats-server/library-routes.ts | 10 +- .../services/stats-server/mining-routes.ts | 26 +- .../services/stats-server/mining-support.ts | 6 +- .../services/stats-server/route-support.ts | 8 +- src/stats-transport-architecture.test.ts | 11 +- 14 files changed, 646 insertions(+), 81 deletions(-) create mode 100644 changes/split-resolvers.md create mode 100644 src/core/services/__tests__/stats-server-mining-support.test.ts diff --git a/changes/split-resolvers.md b/changes/split-resolvers.md new file mode 100644 index 00000000..216ad7a1 --- /dev/null +++ b/changes/split-resolvers.md @@ -0,0 +1,5 @@ +type: fixed +area: stats + +- Validated nested and legacy AnkiConnect settings after splitting the resolver, preserving valid modern overrides while warning and falling back for invalid primitive values. +- Hardened stats routes against malformed IDs and static paths, stalled AniList searches, word-mining media collisions, missing Yomitan bridges, and throwing timing observers. diff --git a/src/config/anki-connect-nplusone-migration.ts b/src/config/anki-connect-nplusone-migration.ts index 5c0b8d76..17a90d1c 100644 --- a/src/config/anki-connect-nplusone-migration.ts +++ b/src/config/anki-connect-nplusone-migration.ts @@ -112,7 +112,16 @@ function buildLegacyNPlusOneMigrationOperations(root: JsoncNode | undefined): { if (!key) continue; const valueNode = propertyValue(property); const value = valueNode ? getNodeValue(valueNode) : undefined; - if (key === 'enabled' || key === 'minSentenceWords') { + if (key === 'enabled' && typeof value === 'boolean') { + canonicalNPlusOneValues.set(key, value); + continue; + } + if ( + key === 'minSentenceWords' && + typeof value === 'number' && + Number.isInteger(value) && + value > 0 + ) { canonicalNPlusOneValues.set(key, value); continue; } diff --git a/src/config/resolve/anki-connect.test.ts b/src/config/resolve/anki-connect.test.ts index e1f1afbf..7990936a 100644 --- a/src/config/resolve/anki-connect.test.ts +++ b/src/config/resolve/anki-connect.test.ts @@ -40,6 +40,74 @@ test('modern invalid knownWords.highlightEnabled warns modern key and does not f ); }); +test('invalid modern known-words primitive values warn and keep defaults', () => { + const { context, warnings } = makeContext({ + knownWords: { + refreshMinutes: 'daily', + matchMode: false, + }, + nPlusOne: { + minSentenceWords: 'three', + }, + }); + + applyAnkiConnectResolution(context); + + assert.equal( + context.resolved.ankiConnect.knownWords.refreshMinutes, + DEFAULT_CONFIG.ankiConnect.knownWords.refreshMinutes, + ); + assert.equal( + context.resolved.ankiConnect.knownWords.matchMode, + DEFAULT_CONFIG.ankiConnect.knownWords.matchMode, + ); + assert.equal( + context.resolved.ankiConnect.nPlusOne.minSentenceWords, + DEFAULT_CONFIG.ankiConnect.nPlusOne.minSentenceWords, + ); + assert.deepEqual( + warnings.map((warning) => warning.path), + [ + 'ankiConnect.knownWords.refreshMinutes', + 'ankiConnect.nPlusOne.minSentenceWords', + 'ankiConnect.knownWords.matchMode', + ], + ); +}); + +test('invalid legacy known-words primitive values warn and keep defaults', () => { + const { context, warnings } = makeContext({ + behavior: { + nPlusOneHighlightEnabled: 'yes', + nPlusOneRefreshMinutes: 'daily', + nPlusOneMatchMode: false, + }, + }); + + applyAnkiConnectResolution(context); + + assert.equal( + context.resolved.ankiConnect.knownWords.highlightEnabled, + DEFAULT_CONFIG.ankiConnect.knownWords.highlightEnabled, + ); + assert.equal( + context.resolved.ankiConnect.knownWords.refreshMinutes, + DEFAULT_CONFIG.ankiConnect.knownWords.refreshMinutes, + ); + assert.equal( + context.resolved.ankiConnect.knownWords.matchMode, + DEFAULT_CONFIG.ankiConnect.knownWords.matchMode, + ); + assert.deepEqual( + warnings.map((warning) => warning.path), + [ + 'ankiConnect.behavior.nPlusOneHighlightEnabled', + 'ankiConnect.behavior.nPlusOneRefreshMinutes', + 'ankiConnect.behavior.nPlusOneMatchMode', + ], + ); +}); + test('known-words resolution can run independently from other Anki domains', () => { const { context, warnings } = makeContext({ knownWords: { highlightEnabled: true }, @@ -194,6 +262,40 @@ test('accepts ankiConnect.media.syncAnimatedImageToWordAudio override', () => { ); }); +test('invalid modern Anki subtrees warn and keep resolved defaults', () => { + const { context, warnings } = makeContext({ + fields: { word: 7 }, + media: { generateAudio: 'yes' }, + behavior: { overwriteAudio: 'yes' }, + metadata: { pattern: false }, + }); + + applyAnkiConnectResolution(context); + + assert.equal(context.resolved.ankiConnect.fields.word, DEFAULT_CONFIG.ankiConnect.fields.word); + assert.equal( + context.resolved.ankiConnect.media.generateAudio, + DEFAULT_CONFIG.ankiConnect.media.generateAudio, + ); + assert.equal( + context.resolved.ankiConnect.behavior.overwriteAudio, + DEFAULT_CONFIG.ankiConnect.behavior.overwriteAudio, + ); + assert.equal( + context.resolved.ankiConnect.metadata.pattern, + DEFAULT_CONFIG.ankiConnect.metadata.pattern, + ); + assert.deepEqual( + warnings.map((warning) => warning.path), + [ + 'ankiConnect.fields.word', + 'ankiConnect.media.generateAudio', + 'ankiConnect.behavior.overwriteAudio', + 'ankiConnect.metadata.pattern', + ], + ); +}); + test('maps legacy ankiConnect.wordField to modern ankiConnect.fields.word', () => { const { context, warnings } = makeContext({ wordField: 'TargetWordLegacy', diff --git a/src/config/resolve/anki-connect/initialize.ts b/src/config/resolve/anki-connect/initialize.ts index 854c41a0..fac850f0 100644 --- a/src/config/resolve/anki-connect/initialize.ts +++ b/src/config/resolve/anki-connect/initialize.ts @@ -52,33 +52,21 @@ export function initializeAnkiConnectResolution( : {}), fields: { ...context.resolved.ankiConnect.fields, - ...(isObject(ankiConnect.fields) - ? (ankiConnect.fields as (typeof context.resolved)['ankiConnect']['fields']) - : {}), }, media: { ...context.resolved.ankiConnect.media, - ...(isObject(ankiConnect.media) - ? (ankiConnect.media as (typeof context.resolved)['ankiConnect']['media']) - : {}), }, knownWords: { ...context.resolved.ankiConnect.knownWords, }, behavior: { ...context.resolved.ankiConnect.behavior, - ...(isObject(ankiConnect.behavior) - ? (ankiConnect.behavior as (typeof context.resolved)['ankiConnect']['behavior']) - : {}), }, proxy: { ...context.resolved.ankiConnect.proxy, }, metadata: { ...context.resolved.ankiConnect.metadata, - ...(isObject(ankiConnect.metadata) - ? (ankiConnect.metadata as (typeof context.resolved)['ankiConnect']['metadata']) - : {}), }, isLapis: { ...context.resolved.ankiConnect.isLapis, diff --git a/src/config/resolve/anki-connect/known-words.ts b/src/config/resolve/anki-connect/known-words.ts index c0c0a392..31bbf5c6 100644 --- a/src/config/resolve/anki-connect/known-words.ts +++ b/src/config/resolve/anki-connect/known-words.ts @@ -1,6 +1,7 @@ import { DEFAULT_CONFIG } from '../../definitions'; import type { ResolveContext } from '../context'; import { asBoolean, asColor, asNumber, asString, isObject } from '../shared'; +import { hasOwn } from './shared'; export function applyAnkiKnownWordsResolution( context: ResolveContext, @@ -13,7 +14,7 @@ export function applyAnkiKnownWordsResolution( const knownWordsHighlightEnabled = asBoolean(knownWordsConfig.highlightEnabled); if (knownWordsHighlightEnabled !== undefined) { context.resolved.ankiConnect.knownWords.highlightEnabled = knownWordsHighlightEnabled; - } else if (knownWordsConfig.highlightEnabled !== undefined) { + } else if (hasOwn(knownWordsConfig, 'highlightEnabled')) { context.warn( 'ankiConnect.knownWords.highlightEnabled', knownWordsConfig.highlightEnabled, @@ -33,6 +34,15 @@ export function applyAnkiKnownWordsResolution( DEFAULT_CONFIG.ankiConnect.knownWords.highlightEnabled, 'Legacy key is deprecated; use ankiConnect.knownWords.highlightEnabled', ); + } else if (hasOwn(behavior, 'nPlusOneHighlightEnabled')) { + context.warn( + 'ankiConnect.behavior.nPlusOneHighlightEnabled', + behavior.nPlusOneHighlightEnabled, + DEFAULT_CONFIG.ankiConnect.knownWords.highlightEnabled, + 'Expected boolean.', + ); + context.resolved.ankiConnect.knownWords.highlightEnabled = + DEFAULT_CONFIG.ankiConnect.knownWords.highlightEnabled; } else { context.resolved.ankiConnect.knownWords.highlightEnabled = DEFAULT_CONFIG.ankiConnect.knownWords.highlightEnabled; @@ -44,7 +54,7 @@ export function applyAnkiKnownWordsResolution( knownWordsRefreshMinutes !== undefined && Number.isInteger(knownWordsRefreshMinutes) && knownWordsRefreshMinutes > 0; - if (knownWordsRefreshMinutes !== undefined) { + if (hasOwn(knownWordsConfig, 'refreshMinutes')) { if (hasValidKnownWordsRefreshMinutes) { context.resolved.ankiConnect.knownWords.refreshMinutes = knownWordsRefreshMinutes; } else { @@ -57,7 +67,7 @@ export function applyAnkiKnownWordsResolution( context.resolved.ankiConnect.knownWords.refreshMinutes = DEFAULT_CONFIG.ankiConnect.knownWords.refreshMinutes; } - } else if (asNumber(behavior.nPlusOneRefreshMinutes) !== undefined) { + } else if (hasOwn(behavior, 'nPlusOneRefreshMinutes')) { const legacyBehaviorNPlusOneRefreshMinutes = asNumber(behavior.nPlusOneRefreshMinutes); const hasValidLegacyRefreshMinutes = legacyBehaviorNPlusOneRefreshMinutes !== undefined && @@ -124,7 +134,7 @@ export function applyAnkiKnownWordsResolution( nPlusOneMinSentenceWords !== undefined && Number.isInteger(nPlusOneMinSentenceWords) && nPlusOneMinSentenceWords > 0; - if (nPlusOneMinSentenceWords !== undefined) { + if (hasOwn(nPlusOneConfig, 'minSentenceWords')) { if (hasValidNPlusOneMinSentenceWords) { context.resolved.ankiConnect.nPlusOne.minSentenceWords = nPlusOneMinSentenceWords; } else { @@ -150,7 +160,7 @@ export function applyAnkiKnownWordsResolution( legacyBehaviorNPlusOneMatchMode === 'headword' || legacyBehaviorNPlusOneMatchMode === 'surface'; if (hasValidKnownWordsMatchMode) { context.resolved.ankiConnect.knownWords.matchMode = knownWordsMatchMode; - } else if (knownWordsMatchMode !== undefined) { + } else if (hasOwn(knownWordsConfig, 'matchMode')) { context.warn( 'ankiConnect.knownWords.matchMode', knownWordsConfig.matchMode, @@ -159,7 +169,7 @@ export function applyAnkiKnownWordsResolution( ); context.resolved.ankiConnect.knownWords.matchMode = DEFAULT_CONFIG.ankiConnect.knownWords.matchMode; - } else if (legacyBehaviorNPlusOneMatchMode !== undefined) { + } else if (hasOwn(behavior, 'nPlusOneMatchMode')) { if (hasValidLegacyMatchMode) { context.resolved.ankiConnect.knownWords.matchMode = legacyBehaviorNPlusOneMatchMode; context.warn( diff --git a/src/config/resolve/anki-connect/modern.ts b/src/config/resolve/anki-connect/modern.ts index 96503db5..51a5fad0 100644 --- a/src/config/resolve/anki-connect/modern.ts +++ b/src/config/resolve/anki-connect/modern.ts @@ -3,43 +3,281 @@ import type { ResolveContext } from '../context'; import { asBoolean, asNumber, asString, isObject } from '../shared'; import { asNotificationType, hasOwn } from './shared'; +function asIntegerInRange(value: unknown, min: number, max: number): number | undefined { + const parsed = asNumber(value); + return parsed !== undefined && Number.isInteger(parsed) && parsed >= min && parsed <= max + ? parsed + : undefined; +} + +function asNonNegativeInteger(value: unknown): number | undefined { + const parsed = asNumber(value); + return parsed !== undefined && Number.isInteger(parsed) && parsed >= 0 ? parsed : undefined; +} + +function asPositiveNumber(value: unknown): number | undefined { + const parsed = asNumber(value); + return parsed !== undefined && parsed > 0 ? parsed : undefined; +} + +function asNonNegativeNumber(value: unknown): number | undefined { + const parsed = asNumber(value); + return parsed !== undefined && parsed >= 0 ? parsed : undefined; +} + +function applyModernValue( + context: ResolveContext, + source: Record, + key: string, + path: string, + parse: (value: unknown) => T | undefined, + fallback: T, + apply: (value: T) => void, + message: string, +): void { + if (!hasOwn(source, key)) return; + const raw = source[key]; + const parsed = parse(raw); + if (parsed === undefined) { + apply(fallback); + context.warn(path, raw, fallback, message); + return; + } + apply(parsed); +} + +function applyModernFieldsResolution( + context: ResolveContext, + fields: Record, +): void { + for (const key of ['word', 'audio', 'image', 'sentence', 'miscInfo', 'translation'] as const) { + applyModernValue( + context, + fields, + key, + `ankiConnect.fields.${key}`, + asString, + DEFAULT_CONFIG.ankiConnect.fields[key], + (value) => { + context.resolved.ankiConnect.fields[key] = value; + }, + 'Expected string.', + ); + } +} + +function applyModernMediaResolution(context: ResolveContext, media: Record): void { + for (const key of [ + 'generateAudio', + 'generateImage', + 'syncAnimatedImageToWordAudio', + 'normalizeAudio', + 'mirrorMpvVolume', + ] as const) { + applyModernValue( + context, + media, + key, + `ankiConnect.media.${key}`, + asBoolean, + DEFAULT_CONFIG.ankiConnect.media[key], + (value) => { + context.resolved.ankiConnect.media[key] = value; + }, + 'Expected boolean.', + ); + } + + applyModernValue( + context, + media, + 'imageType', + 'ankiConnect.media.imageType', + (value) => (value === 'static' || value === 'avif' ? value : undefined), + DEFAULT_CONFIG.ankiConnect.media.imageType, + (value) => { + context.resolved.ankiConnect.media.imageType = value; + }, + "Expected 'static' or 'avif'.", + ); + applyModernValue( + context, + media, + 'imageFormat', + 'ankiConnect.media.imageFormat', + (value) => (value === 'jpg' || value === 'png' || value === 'webp' ? value : undefined), + DEFAULT_CONFIG.ankiConnect.media.imageFormat, + (value) => { + context.resolved.ankiConnect.media.imageFormat = value; + }, + "Expected 'jpg', 'png', or 'webp'.", + ); + applyModernValue( + context, + media, + 'imageQuality', + 'ankiConnect.media.imageQuality', + (value) => asIntegerInRange(value, 1, 100), + DEFAULT_CONFIG.ankiConnect.media.imageQuality, + (value) => { + context.resolved.ankiConnect.media.imageQuality = value; + }, + 'Expected integer between 1 and 100.', + ); + + for (const key of [ + 'imageMaxWidth', + 'imageMaxHeight', + 'animatedMaxWidth', + 'animatedMaxHeight', + ] as const) { + applyModernValue( + context, + media, + key, + `ankiConnect.media.${key}`, + asNonNegativeInteger, + DEFAULT_CONFIG.ankiConnect.media[key] ?? 0, + (value) => { + context.resolved.ankiConnect.media[key] = value; + }, + 'Expected non-negative integer.', + ); + } + + applyModernValue( + context, + media, + 'animatedFps', + 'ankiConnect.media.animatedFps', + (value) => asIntegerInRange(value, 1, 60), + DEFAULT_CONFIG.ankiConnect.media.animatedFps, + (value) => { + context.resolved.ankiConnect.media.animatedFps = value; + }, + 'Expected integer between 1 and 60.', + ); + applyModernValue( + context, + media, + 'animatedCrf', + 'ankiConnect.media.animatedCrf', + (value) => asIntegerInRange(value, 0, 63), + DEFAULT_CONFIG.ankiConnect.media.animatedCrf, + (value) => { + context.resolved.ankiConnect.media.animatedCrf = value; + }, + 'Expected integer between 0 and 63.', + ); + applyModernValue( + context, + media, + 'audioPadding', + 'ankiConnect.media.audioPadding', + asNonNegativeNumber, + DEFAULT_CONFIG.ankiConnect.media.audioPadding, + (value) => { + context.resolved.ankiConnect.media.audioPadding = value; + }, + 'Expected non-negative number.', + ); + + for (const key of ['fallbackDuration', 'maxMediaDuration'] as const) { + applyModernValue( + context, + media, + key, + `ankiConnect.media.${key}`, + asPositiveNumber, + DEFAULT_CONFIG.ankiConnect.media[key], + (value) => { + context.resolved.ankiConnect.media[key] = value; + }, + 'Expected positive number.', + ); + } +} + +function applyModernBehaviorResolution( + context: ResolveContext, + behavior: Record, +): void { + for (const key of [ + 'overwriteAudio', + 'overwriteImage', + 'highlightWord', + 'autoUpdateNewCards', + ] as const) { + applyModernValue( + context, + behavior, + key, + `ankiConnect.behavior.${key}`, + asBoolean, + DEFAULT_CONFIG.ankiConnect.behavior[key], + (value) => { + context.resolved.ankiConnect.behavior[key] = value; + }, + 'Expected boolean.', + ); + } + + applyModernValue( + context, + behavior, + 'mediaInsertMode', + 'ankiConnect.behavior.mediaInsertMode', + (value) => (value === 'append' || value === 'prepend' ? value : undefined), + DEFAULT_CONFIG.ankiConnect.behavior.mediaInsertMode, + (value) => { + context.resolved.ankiConnect.behavior.mediaInsertMode = value; + }, + "Expected 'append' or 'prepend'.", + ); + applyModernValue( + context, + behavior, + 'notificationType', + 'ankiConnect.behavior.notificationType', + asNotificationType, + DEFAULT_CONFIG.ankiConnect.behavior.notificationType, + (value) => { + context.resolved.ankiConnect.behavior.notificationType = value; + }, + "Expected 'overlay', 'system', 'both', 'none', 'osd', or 'osd-system'.", + ); +} + +function applyModernMetadataResolution( + context: ResolveContext, + metadata: Record, +): void { + applyModernValue( + context, + metadata, + 'pattern', + 'ankiConnect.metadata.pattern', + asString, + DEFAULT_CONFIG.ankiConnect.metadata.pattern, + (value) => { + context.resolved.ankiConnect.metadata.pattern = value; + }, + 'Expected string.', + ); +} + export function applyAnkiModernResolution( context: ResolveContext, ankiConnect: Record, behavior: Record, media: Record, ): void { - if (hasOwn(media, 'mirrorMpvVolume')) { - const parsed = asBoolean(media.mirrorMpvVolume); - if (parsed === undefined) { - context.resolved.ankiConnect.media.mirrorMpvVolume = - DEFAULT_CONFIG.ankiConnect.media.mirrorMpvVolume; - context.warn( - 'ankiConnect.media.mirrorMpvVolume', - media.mirrorMpvVolume, - context.resolved.ankiConnect.media.mirrorMpvVolume, - 'Expected boolean.', - ); - } else { - context.resolved.ankiConnect.media.mirrorMpvVolume = parsed; - } - } - - if (hasOwn(behavior, 'notificationType')) { - const parsed = asNotificationType(behavior.notificationType); - if (parsed === undefined) { - context.resolved.ankiConnect.behavior.notificationType = - DEFAULT_CONFIG.ankiConnect.behavior.notificationType; - context.warn( - 'ankiConnect.behavior.notificationType', - behavior.notificationType, - context.resolved.ankiConnect.behavior.notificationType, - "Expected 'overlay', 'system', 'both', 'none', 'osd', or 'osd-system'.", - ); - } else { - context.resolved.ankiConnect.behavior.notificationType = parsed; - } - } + const fields = isObject(ankiConnect.fields) ? ankiConnect.fields : {}; + const metadata = isObject(ankiConnect.metadata) ? ankiConnect.metadata : {}; + applyModernFieldsResolution(context, fields); + applyModernMediaResolution(context, media); + applyModernBehaviorResolution(context, behavior); + applyModernMetadataResolution(context, metadata); applyLapisResolution(context, ankiConnect); applyProxyResolution(context, ankiConnect); diff --git a/src/core/services/__tests__/stats-server-mining-support.test.ts b/src/core/services/__tests__/stats-server-mining-support.test.ts new file mode 100644 index 00000000..640c7a6f --- /dev/null +++ b/src/core/services/__tests__/stats-server-mining-support.test.ts @@ -0,0 +1,31 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { createStatsMiningContext } from '../stats-server/mining-support.js'; + +test('mining timing observer errors do not replace successful phase results', async () => { + const { timeMiningPhase } = createStatsMiningContext({ + onMiningTiming: () => { + throw new Error('observer failed'); + }, + }); + + const result = await timeMiningPhase('word', 'test', async () => 42); + + assert.equal(result, 42); +}); + +test('mining timing observer errors do not replace phase errors', async () => { + const phaseError = new Error('phase failed'); + const { timeMiningPhase } = createStatsMiningContext({ + onMiningTiming: () => { + throw new Error('observer failed'); + }, + }); + + await assert.rejects( + timeMiningPhase('word', 'test', async () => { + throw phaseError; + }), + (error: unknown) => error === phaseError, + ); +}); diff --git a/src/core/services/__tests__/stats-server.test.ts b/src/core/services/__tests__/stats-server.test.ts index 81e4ab46..f8d1b706 100644 --- a/src/core/services/__tests__/stats-server.test.ts +++ b/src/core/services/__tests__/stats-server.test.ts @@ -900,9 +900,11 @@ describe('stats server API routes', () => { }), ); - const res = await app.request('/api/stats/vocabulary?excludePos=particle,auxiliary'); + const res = await app.request( + '/api/stats/vocabulary?excludePos=particle,%20auxiliary,%20,%20noun%20', + ); assert.equal(res.status, 200); - assert.deepEqual(seenArgs, [100, ['particle', 'auxiliary']]); + assert.deepEqual(seenArgs, [100, ['particle', 'auxiliary', 'noun']]); }); it('GET /api/stats/vocabulary returns POS fields', async () => { @@ -1001,6 +1003,36 @@ describe('stats server API routes', () => { assert.equal(res.status, 404); }); + it('PATCH /api/stats/anime/:animeId/anilist accepts only positive integer AniList ids', async () => { + const assignments: Array<{ animeId: number; body: unknown }> = []; + const app = createStatsApp( + createMockTracker({ + reassignAnimeAnilist: async (animeId: number, body: unknown) => { + assignments.push({ animeId, body }); + }, + }), + ); + + for (const anilistId of [-1, 0, 1.5, '12', true, undefined]) { + const res = await app.request('/api/stats/anime/1/anilist', { + method: 'PATCH', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ anilistId }), + }); + assert.equal(res.status, 400, `accepted invalid AniList id: ${String(anilistId)}`); + } + assert.deepEqual(assignments, []); + + const body = { anilistId: 21_802, titleRomaji: 'Little Witch Academia' }; + const res = await app.request('/api/stats/anime/1/anilist', { + method: 'PATCH', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(body), + }); + assert.equal(res.status, 200); + assert.deepEqual(assignments, [{ animeId: 1, body }]); + }); + it('GET /api/stats/anime/:animeId/cover returns cover art', async () => { const app = createStatsApp(createMockTracker()); const res = await app.request('/api/stats/anime/1/cover'); @@ -1348,6 +1380,86 @@ describe('stats server API routes', () => { }); }); + it('POST /api/stats/mine-card requires a non-empty word in word mode', async () => { + await withTempDir(async (dir) => { + const sourcePath = path.join(dir, 'episode.mkv'); + fs.writeFileSync(sourcePath, 'fake media'); + let yomitanCalls = 0; + let mediaCalls = 0; + const app = createStatsApp(createMockTracker(), { + addYomitanNote: async () => { + yomitanCalls += 1; + return 777; + }, + createMediaGenerator: () => ({ + generateAudio: async () => { + mediaCalls += 1; + return Buffer.from('audio'); + }, + generateScreenshot: async () => { + mediaCalls += 1; + return Buffer.from('image'); + }, + generateAnimatedImage: async () => null, + }), + ankiConnectConfig: { media: { generateAudio: true, generateImage: true } }, + }); + + const res = await app.request('/api/stats/mine-card?mode=word', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + sourcePath, + startMs: 1_000, + endMs: 2_000, + sentence: '猫を見た', + word: ' ', + }), + }); + + assert.equal(res.status, 400); + assert.equal(yomitanCalls, 0); + assert.equal(mediaCalls, 0); + }); + }); + + it('POST /api/stats/mine-card does not start word media without a Yomitan bridge', async () => { + await withTempDir(async (dir) => { + const sourcePath = path.join(dir, 'episode.mkv'); + fs.writeFileSync(sourcePath, 'fake media'); + let mediaCalls = 0; + const app = createStatsApp(createMockTracker(), { + createMediaGenerator: () => ({ + generateAudio: async () => { + mediaCalls += 1; + return Buffer.from('audio'); + }, + generateScreenshot: async () => { + mediaCalls += 1; + return Buffer.from('image'); + }, + generateAnimatedImage: async () => null, + }), + ankiConnectConfig: { media: { generateAudio: true, generateImage: true } }, + }); + + const res = await app.request('/api/stats/mine-card?mode=word', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + sourcePath, + startMs: 1_000, + endMs: 2_000, + sentence: '猫を見た', + word: '猫', + }), + }); + + assert.equal(res.status, 500); + assert.equal(mediaCalls, 0); + }); + }); + it('POST /api/stats/mine-card falls back to Default deck for empty deck config', async () => { await withTempDir(async (dir) => { const sourcePath = path.join(dir, 'episode.mkv'); @@ -2155,7 +2267,7 @@ Aligned English subtitle const updateRequest = requests.find((request) => request.action === 'updateNoteFields'); const audioValue = updateRequest?.params?.note?.fields?.SentenceAudio; - assert.match(audioValue ?? '', /^\[sound:subminer_audio_\d+\.mp3\]$/); + assert.match(audioValue ?? '', /^\[sound:subminer_audio_\d+_777\.mp3\]$/); assert.equal(updateRequest?.params?.note?.fields?.ExpressionAudio, undefined); }); }); @@ -2279,8 +2391,8 @@ Aligned English subtitle const updateRequest = requests.find((request) => request.action === 'updateNoteFields'); const fields = updateRequest?.params?.note?.fields ?? {}; - assert.match(fields.SentenceAudio ?? '', /^\[sound:subminer_audio_\d+\.mp3\]$/); - assert.match(fields.Picture ?? '', /^$/); + assert.match(fields.SentenceAudio ?? '', /^\[sound:subminer_audio_\d+_777\.mp3\]$/); + assert.match(fields.Picture ?? '', /^$/); assert.equal(fields.ExpressionAudio, undefined); assert.equal(fields.SelectionText, undefined); }); @@ -2335,8 +2447,8 @@ Aligned English subtitle const updateRequest = requests.find((request) => request.action === 'updateNoteFields'); const fields = updateRequest?.params?.note?.fields ?? {}; - assert.match(fields.SentenceAudio ?? '', /^\[sound:subminer_audio_\d+\.mp3\]$/); - assert.match(fields.Picture ?? '', /^$/); + assert.match(fields.SentenceAudio ?? '', /^\[sound:subminer_audio_\d+_777\.mp3\]$/); + assert.match(fields.Picture ?? '', /^$/); assert.equal(fields.ExpressionAudio, undefined); assert.equal(fields.SelectionText, undefined); }); @@ -2492,7 +2604,7 @@ Aligned English subtitle const updateRequest = requests.find((request) => request.action === 'updateNoteFields'); assert.match( updateRequest?.params?.note?.fields?.Voice ?? '', - /^\[sound:subminer_audio_\d+\.mp3\]$/, + /^\[sound:subminer_audio_\d+_12345\.mp3\]$/, ); }, { notesInfoFields: null }, @@ -2580,7 +2692,11 @@ Aligned English subtitle const updateRequest = requests.find((request) => request.action === 'updateNoteFields'); const audioValue = updateRequest?.params?.note?.fields?.SentenceAudio; - assert.match(audioValue ?? '', /^\[sound:subminer_audio_\d+\.mp3\]$/); + assert.match(audioValue ?? '', /^\[sound:subminer_audio_\d+_12345\.mp3\]$/); + assert.match( + updateRequest?.params?.note?.fields?.Picture ?? '', + /^$/, + ); assert.equal(updateRequest?.params?.note?.fields?.ExpressionAudio, undefined); }); }); @@ -2670,6 +2786,26 @@ Aligned English subtitle assert.deepEqual(await res.json(), { ok: true }); }); + it('DELETE /api/stats/sessions rejects non-safe or fractional session ids', async () => { + let deleteCalls = 0; + const app = createStatsApp( + createMockTracker({ + deleteSessions: async () => { + deleteCalls += 1; + }, + }), + ); + + const res = await app.request('/api/stats/sessions', { + method: 'DELETE', + headers: { 'Content-Type': 'application/json' }, + body: '{"sessionIds":[1.5,1e309,9007199254740992]}', + }); + + assert.equal(res.status, 400); + assert.equal(deleteCalls, 0); + }); + it('POST /api/stats/anki/browse returns 400 for missing noteId', async () => { const app = createStatsApp(createMockTracker()); const res = await app.request('/api/stats/anki/browse', { method: 'POST' }); @@ -2694,8 +2830,10 @@ Aligned English subtitle const originalFetch = globalThis.fetch; let acquireCalls = 0; let recordCalls = 0; - globalThis.fetch = (async () => - new Response( + let requestSignal: AbortSignal | null | undefined; + globalThis.fetch = (async (_input: RequestInfo | URL, init?: RequestInit) => { + requestSignal = init?.signal; + return new Response( JSON.stringify({ data: { Page: { @@ -2707,7 +2845,8 @@ Aligned English subtitle status: 200, headers: { 'Content-Type': 'application/json', 'X-RateLimit-Remaining': '29' }, }, - )) as typeof fetch; + ); + }) as typeof fetch; try { const app = createStatsApp(createMockTracker(), { @@ -2725,6 +2864,7 @@ Aligned English subtitle assert.equal(res.status, 200); assert.equal(acquireCalls, 1); assert.equal(recordCalls, 1); + assert.ok(requestSignal instanceof AbortSignal); } finally { globalThis.fetch = originalFetch; } @@ -2923,6 +3063,15 @@ Aligned English subtitle }); }); + it('returns not found for malformed percent-encoded asset paths', async () => { + await withTempDir(async (dir) => { + const app = createStatsApp(createMockTracker(), { staticDir: dir }); + const res = await app.request('/assets/%'); + + assert.equal(res.status, 404); + }); + }); + it('fetches and serves missing cover art on demand', async () => { let ensureCalls = 0; let hasCover = false; diff --git a/src/core/services/stats-server/integration-routes.ts b/src/core/services/stats-server/integration-routes.ts index d25ac179..cdc26510 100644 --- a/src/core/services/stats-server/integration-routes.ts +++ b/src/core/services/stats-server/integration-routes.ts @@ -17,6 +17,7 @@ import { } from './route-support.js'; const ANKI_CONNECT_FETCH_TIMEOUT_MS = 3_000; +const ANILIST_FETCH_TIMEOUT_MS = 3_000; export function registerStatsIntegrationRoutes( app: Hono, @@ -39,6 +40,7 @@ export function registerStatsIntegrationRoutes( const res = await fetch('https://graphql.anilist.co', { method: 'POST', headers: { 'Content-Type': 'application/json' }, + signal: AbortSignal.timeout(ANILIST_FETCH_TIMEOUT_MS), body: JSON.stringify({ query: `query ($search: String!) { Page(perPage: 10) { @@ -124,7 +126,13 @@ export function registerStatsIntegrationRoutes( const animeId = parseIntQuery(c.req.param('animeId'), 0); if (animeId <= 0) return c.body(null, 400); const body = await c.req.json().catch(() => null); - if (!body?.anilistId) return c.body(null, 400); + if ( + typeof body?.anilistId !== 'number' || + !Number.isInteger(body.anilistId) || + body.anilistId <= 0 + ) { + return c.body(null, 400); + } await tracker.reassignAnimeAnilist(animeId, body); return c.json(statsJson('reassignAnimeAnilist', { ok: true })); }); diff --git a/src/core/services/stats-server/library-routes.ts b/src/core/services/stats-server/library-routes.ts index c2fd426f..18dfeada 100644 --- a/src/core/services/stats-server/library-routes.ts +++ b/src/core/services/stats-server/library-routes.ts @@ -19,7 +19,11 @@ export function registerStatsLibraryRoutes( ): void { app.get('/api/stats/vocabulary', async (c) => { const limit = parseIntQuery(c.req.query('limit'), 100, 500); - const excludePos = c.req.query('excludePos')?.split(',').filter(Boolean); + const excludePos = c.req + .query('excludePos') + ?.split(',') + .map((entry) => entry.trim()) + .filter(Boolean); const vocab = await tracker.getVocabularyStats(limit, excludePos); return c.json(statsJson('vocabulary', vocab)); }); @@ -164,7 +168,9 @@ export function registerStatsLibraryRoutes( app.delete('/api/stats/sessions', async (c) => { const body = await c.req.json().catch(() => null); const ids = Array.isArray(body?.sessionIds) - ? body.sessionIds.filter((id: unknown) => typeof id === 'number' && id > 0) + ? body.sessionIds.filter( + (id: unknown): id is number => Number.isSafeInteger(id) && (id as number) > 0, + ) : []; if (ids.length === 0) return c.body(null, 400); await tracker.deleteSessions(ids); diff --git a/src/core/services/stats-server/mining-routes.ts b/src/core/services/stats-server/mining-routes.ts index 260a976f..690ac13f 100644 --- a/src/core/services/stats-server/mining-routes.ts +++ b/src/core/services/stats-server/mining-routes.ts @@ -43,7 +43,13 @@ export function registerStatsMiningRoutes(app: Hono, options?: StatsMiningRouteO const rawMode = c.req.query('mode'); const mode = rawMode === 'audio' ? 'audio' : rawMode === 'word' ? 'word' : 'sentence'; - if (!sourcePath || !sentence || !Number.isFinite(startMs) || !Number.isFinite(endMs)) { + if ( + !sourcePath || + !sentence || + (mode === 'word' && !word) || + !Number.isFinite(startMs) || + !Number.isFinite(endMs) + ) { return c.json( statsJson('mineCard', { error: 'sourcePath, sentence, startMs, and endMs are required', @@ -63,6 +69,10 @@ export function registerStatsMiningRoutes(app: Hono, options?: StatsMiningRouteO if (!ankiConfig) { return c.json(statsJson('mineCard', { error: 'AnkiConnect is not configured' }), 500); } + const addYomitanNote = options?.addYomitanNote; + if (mode === 'word' && !addYomitanNote) { + return c.json(statsJson('mineCard', { error: 'Yomitan bridge not available' }), 500); + } const secondarySubtitleLanguages = getSecondarySubtitleLanguages(); let retimedSecondaryText = ''; if (mode === 'sentence' && !bodySecondaryText) { @@ -188,15 +198,11 @@ export function registerStatsMiningRoutes(app: Hono, options?: StatsMiningRouteO }; if (mode === 'word') { - if (!options?.addYomitanNote) { - return c.json(statsJson('mineCard', { error: 'Yomitan bridge not available' }), 500); - } - const [yomitanResult, audioResult, imageResult] = await Promise.allSettled([ timeMiningPhase( 'word', 'addYomitanNote', - () => options.addYomitanNote!(word), + () => addYomitanNote!(word), (noteId) => (typeof noteId === 'number' ? { noteId } : {}), ), audioPromise, @@ -269,7 +275,7 @@ export function registerStatsMiningRoutes(app: Hono, options?: StatsMiningRouteO applyStatsWordAndSentenceCardFields(mediaFields, noteInfo, ankiConfig); if (audioBuffer) { - const audioFilename = `subminer_audio_${timestamp}.mp3`; + const audioFilename = `subminer_audio_${timestamp}_${noteId}.mp3`; try { await timeMiningPhase('word', 'uploadAudio', () => client.storeMediaFile(audioFilename, audioBuffer), @@ -282,7 +288,7 @@ export function registerStatsMiningRoutes(app: Hono, options?: StatsMiningRouteO if (imageBuffer) { const imageExt = imageType === 'avif' ? 'avif' : (ankiConfig.media?.imageFormat ?? 'jpg'); - const imageFilename = `subminer_image_${timestamp}.${imageExt}`; + const imageFilename = `subminer_image_${timestamp}_${noteId}.${imageExt}`; try { await timeMiningPhase('word', 'uploadImage', () => client.storeMediaFile(imageFilename, imageBuffer), @@ -402,7 +408,7 @@ export function registerStatsMiningRoutes(app: Hono, options?: StatsMiningRouteO } if (audioBuffer) { - const audioFilename = `subminer_audio_${timestamp}.mp3`; + const audioFilename = `subminer_audio_${timestamp}_${noteId}.mp3`; try { await timeMiningPhase(mode, 'uploadAudio', () => client.storeMediaFile(audioFilename, audioBuffer), @@ -418,7 +424,7 @@ export function registerStatsMiningRoutes(app: Hono, options?: StatsMiningRouteO if (imageBuffer) { const imageExt = imageType === 'avif' ? 'avif' : (ankiConfig.media?.imageFormat ?? 'jpg'); - const imageFilename = `subminer_image_${timestamp}.${imageExt}`; + const imageFilename = `subminer_image_${timestamp}_${noteId}.${imageExt}`; try { await timeMiningPhase(mode, 'uploadImage', () => client.storeMediaFile(imageFilename, imageBuffer), diff --git a/src/core/services/stats-server/mining-support.ts b/src/core/services/stats-server/mining-support.ts index 5693c9dd..9a98b318 100644 --- a/src/core/services/stats-server/mining-support.ts +++ b/src/core/services/stats-server/mining-support.ts @@ -153,7 +153,11 @@ export function createStatsMiningContext(options?: StatsMiningRouteOptions) { } }; const recordMiningTiming = (event: StatsMiningTimingEvent): void => { - options?.onMiningTiming?.(event); + try { + options?.onMiningTiming?.(event); + } catch { + // Timing observers must not affect mining execution. + } statsMiningLogger.debug( `[stats:mining] ${event.mode} ${event.phase} ${Math.round(event.elapsedMs)}ms`, event, diff --git a/src/core/services/stats-server/route-support.ts b/src/core/services/stats-server/route-support.ts index 46bbb4d8..d0050cd6 100644 --- a/src/core/services/stats-server/route-support.ts +++ b/src/core/services/stats-server/route-support.ts @@ -225,7 +225,13 @@ export function buildAnkiNotePreview( function resolveStatsStaticPath(staticDir: string, requestPath: string): string | null { const normalizedPath = requestPath.replace(/^\/+/, '') || 'index.html'; const absoluteStaticDir = resolve(staticDir); - const absolutePath = resolve(absoluteStaticDir, decodeURIComponent(normalizedPath)); + let decodedPath: string; + try { + decodedPath = decodeURIComponent(normalizedPath); + } catch { + return null; + } + const absolutePath = resolve(absoluteStaticDir, decodedPath); if ( absolutePath !== absoluteStaticDir && !absolutePath.startsWith(`${absoluteStaticDir}${sep}`) diff --git a/src/stats-transport-architecture.test.ts b/src/stats-transport-architecture.test.ts index 3604ff2d..8828b7d2 100644 --- a/src/stats-transport-architecture.test.ts +++ b/src/stats-transport-architecture.test.ts @@ -28,9 +28,12 @@ test('stats data uses the shared HTTP contract while native dialogs retain IPC', 'integration-routes.ts', 'library-routes.ts', 'mining-routes.ts', - ] - .map((filename) => read(`src/core/services/stats-server/${filename}`)) - .join('\n'); + ].map((filename) => ({ + filename, + source: read(`src/core/services/stats-server/${filename}`), + })); assert.match(apiClient, /StatsHttpClient/); - assert.match(statsServerRoutes, /statsJson/); + for (const { filename, source } of statsServerRoutes) { + assert.match(source, /statsJson/, `${filename} must use the shared HTTP contract`); + } });