fix: blocklist-search lookup against queue cache instead of downloadClientRegistry
CI / Security audit (push) Successful in 1m52s
Docs Check / Markdown lint (push) Successful in 1m37s
Build and Push Docker Image / build (push) Successful in 2m2s
Licence Check / Licence compatibility and copyright header verification (push) Failing after 2m33s
CI / Swagger Validation & Coverage (push) Successful in 3m17s
Docs Check / Mermaid diagram parse check (push) Successful in 3m31s
CI / Tests & coverage (push) Successful in 4m5s

Fixes the root cause of the regression from v1.7.16. The v1.7.16 fix
correctly cast arrQueueId to String, but the lookup was performed
against downloadClientRegistry.getAllDownloads() which returns raw
download client data (qBittorrent, SABnzbd, etc.) that never has
arrQueueId populated.

The fix now looks up the queue record directly from the Sonarr/Radarr
queue cache where record.id is the numeric queue ID, using String()
casting on both sides to handle the DOM-dataset (string) vs API
response (number) type difference.

Resolves Gitea Issue #48
Closes #48
This commit is contained in:
2026-05-24 22:48:17 +01:00
parent 83c9d4d164
commit 7690d959b3
19 changed files with 315 additions and 71 deletions
+43 -58
View File
@@ -225,7 +225,7 @@ function invalidatePollCache() {
'poll:sab-queue', 'poll:sab-history',
'poll:sonarr-queue', 'poll:sonarr-history', 'poll:sonarr-tags',
'poll:radarr-queue', 'poll:radarr-history', 'poll:radarr-tags',
'poll:qbittorrent'
'poll:qbittorrent', 'poll:ombi-requests'
];
for (const k of keys) cache.invalidate(k);
}
@@ -749,12 +749,14 @@ describe('POST /api/dashboard/blocklist-search', () => {
const app = createApp({ skipRateLimits: true });
const { cookies, csrf } = await loginAs(app);
const csrfCookie = cookies.find(c => c.startsWith('csrf_token='));
// Mock getAllDownloads to return a download that doesn't qualify for blocklist
const downloadClientRegistry = require('../../server/utils/downloadClients');
const mockGetAllDownloads = vi.spyOn(downloadClientRegistry, 'getAllDownloads').mockResolvedValue([
{ arrQueueId: 1, arrType: 'sonarr', importIssues: [], qbittorrent: null }
]);
// Seed cache: queue record exists but has no import issues (non-admin cannot blocklist)
cache.set('poll:sonarr-queue', { records: [{
id: 1,
title: 'My.Show.S01E01.720p',
trackedDownloadState: 'downloading',
trackedDownloadStatus: 'ok'
}] }, CACHE_TTL);
const res = await request(app)
.post('/api/dashboard/blocklist-search')
@@ -763,18 +765,14 @@ describe('POST /api/dashboard/blocklist-search', () => {
.send({ arrQueueId: 1, arrType: 'sonarr', arrInstanceUrl: SONARR_BASE, arrInstanceKey: 'key', arrContentId: 501, arrContentType: 'episode' });
expect(res.status).toBe(403);
expect(res.body.error).toMatch(/permission denied/i);
mockGetAllDownloads.mockRestore();
});
it('returns 403 for non-admin when download not found in active downloads', async () => {
it('returns 403 for non-admin when download not found in arr queue cache', async () => {
const app = createApp({ skipRateLimits: true });
const { cookies, csrf } = await loginAs(app);
const csrfCookie = cookies.find(c => c.startsWith('csrf_token='));
// Mock getAllDownloads to return empty array (download not found)
const downloadClientRegistry = require('../../server/utils/downloadClients');
const mockGetAllDownloads = vi.spyOn(downloadClientRegistry, 'getAllDownloads').mockResolvedValue([]);
// Cache is already seeded empty by beforeEach; no queue record with id=1 exists
const res = await request(app)
.post('/api/dashboard/blocklist-search')
.set('Cookie', [...cookies, csrfCookie].join('; '))
@@ -782,19 +780,21 @@ describe('POST /api/dashboard/blocklist-search', () => {
.send({ arrQueueId: 1, arrType: 'sonarr', arrInstanceUrl: SONARR_BASE, arrContentId: 501, arrContentType: 'episode' });
expect(res.status).toBe(403);
expect(res.body.error).toMatch(/download not found/i);
mockGetAllDownloads.mockRestore();
});
it('returns 200 for non-admin with import issues (qualifying condition)', async () => {
const app = createApp({ skipRateLimits: true });
const { cookies, csrf } = await loginAs(app);
const csrfCookie = cookies.find(c => c.startsWith('csrf_token='));
// Mock getAllDownloads to return a download with import issues (qualifies for blocklist)
const downloadClientRegistry = require('../../server/utils/downloadClients');
const mockGetAllDownloads = vi.spyOn(downloadClientRegistry, 'getAllDownloads').mockResolvedValue([
{ arrQueueId: 1, arrType: 'sonarr', importIssues: ['Import error 1'], qbittorrent: null }
]);
// Seed cache: queue record with import issues qualifies non-admin for blocklist
cache.set('poll:sonarr-queue', { records: [{
id: 1,
title: 'My.Show.S01E01.720p',
trackedDownloadState: 'importPending',
trackedDownloadStatus: 'warning',
statusMessages: [{ messages: ['Import error 1'] }]
}] }, CACHE_TTL);
// Mock Sonarr DELETE and command endpoints
nock(SONARR_BASE)
@@ -812,7 +812,6 @@ describe('POST /api/dashboard/blocklist-search', () => {
.send({ arrQueueId: 1, arrType: 'sonarr', arrInstanceUrl: SONARR_BASE, arrContentId: 501, arrContentType: 'episode' });
expect(res.status).toBe(200);
expect(res.body.ok).toBe(true);
mockGetAllDownloads.mockRestore();
});
it('returns 400 when required fields are missing', async () => {
@@ -843,11 +842,8 @@ describe('POST /api/dashboard/blocklist-search', () => {
const app = createApp({ skipRateLimits: true });
const { cookies, csrf, csrfCookie } = await getAuthHeaders(app);
// Mock getAllDownloads to return a matching download for admin
const downloadClientRegistry = require('../../server/utils/downloadClients');
const mockGetAllDownloads = vi.spyOn(downloadClientRegistry, 'getAllDownloads').mockResolvedValue([
{ arrQueueId: 1001, arrType: 'sonarr', importIssues: [], qbittorrent: null }
]);
// Seed the Sonarr queue cache so the permission lookup finds the record
cache.set('poll:sonarr-queue', { records: [SONARR_QUEUE_RECORD] }, CACHE_TTL);
nock(SONARR_BASE)
.delete('/api/v3/queue/1001')
@@ -864,18 +860,14 @@ describe('POST /api/dashboard/blocklist-search', () => {
.send({ arrQueueId: 1001, arrType: 'sonarr', arrInstanceUrl: SONARR_BASE, arrInstanceKey: 'sk', arrContentId: 501, arrContentType: 'episode' });
expect(res.status).toBe(200);
expect(res.body.ok).toBe(true);
mockGetAllDownloads.mockRestore();
});
it('calls Radarr DELETE+command and returns ok:true', async () => {
const app = createApp({ skipRateLimits: true });
const { cookies, csrf, csrfCookie } = await getAuthHeaders(app);
// Mock getAllDownloads to return a matching download for admin
const downloadClientRegistry = require('../../server/utils/downloadClients');
const mockGetAllDownloads = vi.spyOn(downloadClientRegistry, 'getAllDownloads').mockResolvedValue([
{ arrQueueId: 2001, arrType: 'radarr', importIssues: [], qbittorrent: null }
]);
// Seed the Radarr queue cache so the permission lookup finds the record
cache.set('poll:radarr-queue', { records: [RADARR_QUEUE_RECORD] }, CACHE_TTL);
nock(RADARR_BASE)
.delete('/api/v3/queue/2001')
@@ -892,18 +884,14 @@ describe('POST /api/dashboard/blocklist-search', () => {
.send({ arrQueueId: 2001, arrType: 'radarr', arrInstanceUrl: RADARR_BASE, arrInstanceKey: 'rk', arrContentId: 99, arrContentType: 'movie' });
expect(res.status).toBe(200);
expect(res.body.ok).toBe(true);
mockGetAllDownloads.mockRestore();
});
it('returns 502 when Sonarr DELETE request fails', async () => {
const app = createApp({ skipRateLimits: true });
const { cookies, csrf, csrfCookie } = await getAuthHeaders(app);
// Mock getAllDownloads to return a matching download for admin
const downloadClientRegistry = require('../../server/utils/downloadClients');
const mockGetAllDownloads = vi.spyOn(downloadClientRegistry, 'getAllDownloads').mockResolvedValue([
{ arrQueueId: 1001, arrType: 'sonarr', importIssues: [], qbittorrent: null }
]);
// Seed the Sonarr queue cache so the permission lookup finds the record
cache.set('poll:sonarr-queue', { records: [SONARR_QUEUE_RECORD] }, CACHE_TTL);
nock(SONARR_BASE)
.delete('/api/v3/queue/1001')
@@ -916,17 +904,14 @@ describe('POST /api/dashboard/blocklist-search', () => {
.set('X-CSRF-Token', csrf)
.send({ arrQueueId: 1001, arrType: 'sonarr', arrInstanceUrl: SONARR_BASE, arrInstanceKey: 'sk', arrContentId: 501, arrContentType: 'episode' });
expect(res.status).toBe(502);
mockGetAllDownloads.mockRestore();
});
it('returns 200 OK when arrContentId is null but arrSeriesId is present (fallback SeriesSearch)', async () => {
const app = createApp({ skipRateLimits: true });
const { cookies, csrf, csrfCookie } = await getAuthHeaders(app);
const downloadClientRegistry = require('../../server/utils/downloadClients');
const mockGetAllDownloads = vi.spyOn(downloadClientRegistry, 'getAllDownloads').mockResolvedValue([
{ arrQueueId: 1001, arrType: 'sonarr', importIssues: [], qbittorrent: null }
]);
// Seed the Sonarr queue cache so the permission lookup finds the record
cache.set('poll:sonarr-queue', { records: [SONARR_QUEUE_RECORD] }, CACHE_TTL);
nock(SONARR_BASE)
.delete('/api/v3/queue/1001')
@@ -943,17 +928,14 @@ describe('POST /api/dashboard/blocklist-search', () => {
.send({ arrQueueId: 1001, arrType: 'sonarr', arrInstanceUrl: SONARR_BASE, arrInstanceKey: 'sk', arrSeriesId: 42, arrContentType: 'episode' });
expect(res.status).toBe(200);
expect(res.body.ok).toBe(true);
mockGetAllDownloads.mockRestore();
});
it('triggers EpisodeSearch with multiple episode IDs when arrContentIds is provided', async () => {
const app = createApp({ skipRateLimits: true });
const { cookies, csrf, csrfCookie } = await getAuthHeaders(app);
const downloadClientRegistry = require('../../server/utils/downloadClients');
const mockGetAllDownloads = vi.spyOn(downloadClientRegistry, 'getAllDownloads').mockResolvedValue([
{ arrQueueId: 1001, arrType: 'sonarr', importIssues: [], qbittorrent: null }
]);
// Seed the Sonarr queue cache so the permission lookup finds the record
cache.set('poll:sonarr-queue', { records: [SONARR_QUEUE_RECORD] }, CACHE_TTL);
nock(SONARR_BASE)
.delete('/api/v3/queue/1001')
@@ -970,21 +952,25 @@ describe('POST /api/dashboard/blocklist-search', () => {
.send({ arrQueueId: 1001, arrType: 'sonarr', arrInstanceUrl: SONARR_BASE, arrInstanceKey: 'sk', arrContentIds: [12, 13, 14], arrContentType: 'episode' });
expect(res.status).toBe(200);
expect(res.body.ok).toBe(true);
mockGetAllDownloads.mockRestore();
});
it('matches download correctly when arrQueueId is sent as a string but stored as a number (type mismatch regression)', async () => {
// Regression test for GitHub #48: arrQueueId from the SPA DOM dataset is always
// a string, but the value stored in allDownloads from the Radarr/Sonarr API is a number.
it('matches download correctly when arrQueueId is sent as a string but stored as a number in queue cache (type mismatch regression)', async () => {
// Regression test for issue #48 (v2): arrQueueId from the SPA DOM dataset is always
// a string, but the queue record id from the Radarr/Sonarr API cache is a number.
// Without String() casting the === comparison fails and returns 403.
const app = createApp({ skipRateLimits: true });
const { cookies, csrf, csrfCookie } = await getAuthHeaders(app);
const downloadClientRegistry = require('../../server/utils/downloadClients');
const mockGetAllDownloads = vi.spyOn(downloadClientRegistry, 'getAllDownloads').mockResolvedValue([
// arrQueueId stored as a number (as Radarr API returns it)
{ arrQueueId: 9050001, arrType: 'radarr', importIssues: [], qbittorrent: null }
]);
// Seed Radarr queue with a numeric id (as Radarr API returns it)
cache.set('poll:radarr-queue', { records: [{
id: 9050001,
title: 'Project.Hail.Mary.2026.2160p',
movieId: 77,
trackedDownloadState: 'downloading',
trackedDownloadStatus: 'ok',
_instanceUrl: RADARR_BASE,
_instanceKey: 'rk'
}] }, CACHE_TTL);
nock(RADARR_BASE)
.delete('/api/v3/queue/9050001')
@@ -1002,7 +988,6 @@ describe('POST /api/dashboard/blocklist-search', () => {
.send({ arrQueueId: '9050001', arrType: 'radarr', arrInstanceUrl: RADARR_BASE, arrInstanceKey: 'rk', arrContentId: 77, arrContentType: 'movie' });
expect(res.status).toBe(200);
expect(res.body.ok).toBe(true);
mockGetAllDownloads.mockRestore();
});
});