From 4a7c9b04f1b804a9c34181f2fa79156f8d236f76 Mon Sep 17 00:00:00 2001 From: sudacode Date: Wed, 12 Aug 2026 00:49:32 -0700 Subject: [PATCH] fix(stats): enforce cleanup mode exclusivity and JSON requests - Reject conflicting explicit cleanup modes - Require application/json for duplicate-line maintenance requests --- docs-site/immersion-tracking.md | 2 +- docs-site/usage.md | 2 +- launcher/config/cli-parser-builder.ts | 10 ++++++-- launcher/parse-args.test.ts | 21 +++++++++++++--- .../services/__tests__/stats-server.test.ts | 24 +++++++++++++++++++ .../services/stats-server/library-routes.ts | 2 ++ 6 files changed, 54 insertions(+), 7 deletions(-) diff --git a/docs-site/immersion-tracking.md b/docs-site/immersion-tracking.md index 76d9b752..4865d12f 100644 --- a/docs-site/immersion-tracking.md +++ b/docs-site/immersion-tracking.md @@ -147,7 +147,7 @@ subminer stats cleanup --duplicate-lines --dry-run --lookback-days 30 subminer stats cleanup --duplicate-lines --lookback-days 30 ``` -`--duplicate-lines` (short: `-d`) picks the cleanup mode, so it cannot be combined with `--lifetime`, and `--dry-run` and `--lookback-days ` only apply to it. Omitting `--lookback-days` scans all history; the value must be at least one day. +`--duplicate-lines` (short: `-d`) picks the cleanup mode, so it cannot be combined with `--vocab` or `--lifetime`, and `--dry-run` and `--lookback-days ` only apply to it. Omitting `--lookback-days` scans all history; the value must be at least one day. Runs never cross a session boundary, so rewatching an episode keeps both watches. Session telemetry (watch time, lines seen, tokens seen) and the rollups derived from it are left as recorded: they are cumulative samples taken during playback, and cannot be recomputed for sessions whose raw rows have since been pruned. diff --git a/docs-site/usage.md b/docs-site/usage.md index 7b5614d2..ad695fa4 100644 --- a/docs-site/usage.md +++ b/docs-site/usage.md @@ -109,7 +109,7 @@ subminer app --stop # Stop the background app subminer --version # Print the launcher's version ``` -`stats cleanup` runs one mode per invocation: `-v`/`--vocab` (the default), `-l`/`--lifetime`, or `-d`/`--duplicate-lines`, and `--lifetime` cannot be combined with `--duplicate-lines`. `--dry-run` and `--lookback-days ` apply to `--duplicate-lines` only and are rejected without it; `--lookback-days` must be at least one day, and leaving it off scans all history. +`stats cleanup` runs one mode per invocation: `-v`/`--vocab` (the default), `-l`/`--lifetime`, or `-d`/`--duplicate-lines`; explicitly selected modes cannot be combined. `--dry-run` and `--lookback-days ` apply to `--duplicate-lines` only and are rejected without it; `--lookback-days` must be at least one day, and leaving it off scans all history. Jellyfin, cross-machine sync, and character-dictionary commands have their own sections: [Jellyfin](/jellyfin-integration), [Sync Between Machines](/launcher-script#sync-between-machines), and [Character Dictionary](/character-dictionary). diff --git a/launcher/config/cli-parser-builder.ts b/launcher/config/cli-parser-builder.ts index dd2bd879..e531123d 100644 --- a/launcher/config/cli-parser-builder.ts +++ b/launcher/config/cli-parser-builder.ts @@ -316,14 +316,20 @@ export function parseCliPrograms( 'Stats --vocab, --lifetime and --duplicate-lines flags require the cleanup action.', ); } - if (options.duplicateLines !== true && (options.dryRun === true || options.lookbackDays)) { + if ( + options.duplicateLines !== true && + (options.dryRun === true || options.lookbackDays !== undefined) + ) { throw new Error('Stats --dry-run and --lookback-days require --duplicate-lines.'); } if (normalizedAction === 'cleanup') { statsCleanup = true; statsCleanupLifetime = options.lifetime === true; statsCleanupDuplicateLines = options.duplicateLines === true; - if (statsCleanupLifetime && statsCleanupDuplicateLines) { + const explicitModeCount = [options.vocab, options.lifetime, options.duplicateLines].filter( + (value) => value === true, + ).length; + if (explicitModeCount > 1) { throw new Error('Stats cleanup runs one mode at a time.'); } // Vocabulary cleanup stays the default so `stats cleanup` keeps its old meaning. diff --git a/launcher/parse-args.test.ts b/launcher/parse-args.test.ts index 90e98142..9dfb5660 100644 --- a/launcher/parse-args.test.ts +++ b/launcher/parse-args.test.ts @@ -262,13 +262,28 @@ test('parseArgs rejects duplicate-line flags without the duplicate-lines mode', assert.match(error.stderr, /--dry-run and --lookback-days require --duplicate-lines/); }); -test('parseArgs rejects combining lifetime and duplicate-line cleanup modes', () => { +test('parseArgs rejects an empty lookback value outside duplicate-line cleanup', () => { const error = withProcessExitIntercept(() => { - parseArgs(['stats', 'cleanup', '--lifetime', '--duplicate-lines'], 'subminer', {}); + parseArgs(['stats', '--lookback-days', ''], 'subminer', {}); }); assert.equal(error.code, 1); - assert.match(error.stderr, /Stats cleanup runs one mode at a time/); + assert.match(error.stderr, /--dry-run and --lookback-days require --duplicate-lines/); +}); + +test('parseArgs rejects combining explicit cleanup modes', () => { + for (const modes of [ + ['--lifetime', '--duplicate-lines'], + ['--vocab', '--duplicate-lines'], + ['--vocab', '--lifetime'], + ]) { + const error = withProcessExitIntercept(() => { + parseArgs(['stats', 'cleanup', ...modes], 'subminer', {}); + }); + + assert.equal(error.code, 1); + assert.match(error.stderr, /Stats cleanup runs one mode at a time/); + } }); test('parseArgs rejects unusable lookback windows', () => { diff --git a/src/core/services/__tests__/stats-server.test.ts b/src/core/services/__tests__/stats-server.test.ts index 7856d81d..c24969e5 100644 --- a/src/core/services/__tests__/stats-server.test.ts +++ b/src/core/services/__tests__/stats-server.test.ts @@ -1064,6 +1064,30 @@ describe('stats server API routes', () => { assert.deepEqual(seenOptions, { dryRun: true, lookbackDays: 30 }); }); + it('POST /api/stats/maintenance/duplicate-lines rejects cross-origin simple requests', async () => { + let cleanupCalls = 0; + const app = createStatsApp( + createMockTracker({ + cleanupDuplicateSubtitleLines: async () => { + cleanupCalls += 1; + throw new Error('cleanup must not run'); + }, + }), + ); + + const res = await app.request('/api/stats/maintenance/duplicate-lines', { + method: 'POST', + headers: { + 'Content-Type': 'text/plain', + Origin: 'https://attacker.example', + }, + body: JSON.stringify({ dryRun: false, lookbackDays: null }), + }); + + assert.equal(res.status, 415); + assert.equal(cleanupCalls, 0); + }); + it('POST /api/stats/maintenance/duplicate-lines rejects a window shorter than a day', async () => { let cleanupCalls = 0; const app = createStatsApp( diff --git a/src/core/services/stats-server/library-routes.ts b/src/core/services/stats-server/library-routes.ts index 650cf36d..357b7c15 100644 --- a/src/core/services/stats-server/library-routes.ts +++ b/src/core/services/stats-server/library-routes.ts @@ -44,6 +44,8 @@ export function registerStatsLibraryRoutes( // Collapse animation bursts older versions recorded frame by frame. `dryRun` measures // the same scan without writing, so the confirmation the user sees is the real cost. app.post('/api/stats/maintenance/duplicate-lines', async (c) => { + const contentType = c.req.header('content-type')?.split(';', 1)[0]?.trim().toLowerCase(); + if (contentType !== 'application/json') return c.body(null, 415); const body = await c.req.json().catch(() => null); const options = parseDuplicateLineCleanupBody(body); if (!options) return c.body(null, 400);