Skip to content

Commit f9ede25

Browse files
authored
Merge pull request #430 from DialmasterOrg/fix/426-permission-error-channel-downloads
fix(#426): use configurable tmp path
2 parents 1d28359 + 185ff04 commit f9ede25

File tree

11 files changed

+78
-8
lines changed

11 files changed

+78
-8
lines changed

.github/workflows/claude.yml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,9 @@ jobs:
4141
- Security implications
4242
- Performance considerations
4343
44-
The PR branch is already checked out. Use `gh pr diff ${{ github.event.pull_request.number }}` to see the changes.
44+
Use `gh pr diff ${{ github.event.pull_request.number }}` to see the changes.
45+
46+
IMPORTANT: Only use `gh` commands. Do NOT run tests, npm commands, or any other shell commands.
4547
4648
Post your review as a comment on the PR using `gh pr comment ${{ github.event.pull_request.number }} --body "your review here"`.
4749
Only post GitHub comments - don't just output text.

server/modules/__tests__/videoValidationModule.test.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
/* eslint-env jest */
22

3+
// Mock tempPathManager before any module that depends on it is loaded
4+
// This prevents configModule's file watcher from starting
5+
jest.mock('../download/tempPathManager', () => ({
6+
getTempBasePath: jest.fn(() => '/tmp/youtarr-downloads'),
7+
}));
8+
39
const videoValidationModule = require('../videoValidationModule');
410
const ytDlpRunner = require('../ytDlpRunner');
511
const archiveModule = require('../archiveModule');

server/modules/__tests__/ytDlpRunner.test.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,10 @@ describe('YtDlpRunner', () => {
8484
};
8585
jest.doMock('../download/ytdlpCommandBuilder', () => ytdlpCommandBuilderMock);
8686

87+
jest.doMock('../download/tempPathManager', () => ({
88+
getTempBasePath: jest.fn(() => '/tmp/youtarr-downloads')
89+
}));
90+
8791
loadModule();
8892
});
8993

server/modules/__tests__/ytdlpModule.test.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ jest.mock('../jobModule', () => ({
1515
getInProgressJobId: jest.fn(),
1616
}));
1717

18+
jest.mock('../download/tempPathManager', () => ({
19+
getTempBasePath: jest.fn(() => '/tmp/youtarr-downloads'),
20+
}));
21+
1822
// Mock logger
1923
jest.mock('../../logger', () => ({
2024
info: jest.fn(),
@@ -393,7 +397,11 @@ describe('ytdlpModule', () => {
393397

394398
const result = await updatePromise;
395399
expect(result.success).toBe(true);
396-
expect(spawn).toHaveBeenCalledWith('yt-dlp', ['-U']);
400+
expect(spawn).toHaveBeenCalledWith('yt-dlp', ['-U'], expect.objectContaining({
401+
env: expect.objectContaining({
402+
TMPDIR: '/tmp/youtarr-downloads'
403+
})
404+
}));
397405
});
398406

399407
it('resets update state after completion', async () => {

server/modules/channelModule.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ const { Op, fn, col, where } = require('sequelize');
1212
const fileCheckModule = require('./fileCheckModule');
1313
const logger = require('../logger');
1414
const { sanitizeNameLikeYtDlp } = require('./filesystem');
15+
const tempPathManager = require('./download/tempPathManager');
1516

1617
const { v4: uuidv4 } = require('uuid');
1718
const { spawn, execSync } = require('child_process');
@@ -151,7 +152,7 @@ class ChannelModule {
151152
const ytDlp = spawn('yt-dlp', args, {
152153
env: {
153154
...process.env,
154-
TMPDIR: '/tmp'
155+
TMPDIR: tempPathManager.getTempBasePath()
155156
}
156157
});
157158

server/modules/download/__tests__/ytdlpCommandBuilder.test.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,20 @@ describe('YtdlpCommandBuilder', () => {
297297
});
298298
});
299299

300+
describe('buildTempPathArgs', () => {
301+
it('should return --paths temp: args with temp base path', () => {
302+
tempPathManager.getTempBasePath.mockReturnValue('/mock/youtube/output/.youtarr_tmp');
303+
const result = YtdlpCommandBuilder.buildTempPathArgs();
304+
expect(result).toEqual(['--paths', 'temp:/mock/youtube/output/.youtarr_tmp']);
305+
});
306+
307+
it('should use external temp path when configured', () => {
308+
tempPathManager.getTempBasePath.mockReturnValue('/tmp/youtarr-downloads');
309+
const result = YtdlpCommandBuilder.buildTempPathArgs();
310+
expect(result).toEqual(['--paths', 'temp:/tmp/youtarr-downloads']);
311+
});
312+
});
313+
300314
describe('buildSubtitleArgs', () => {
301315
it('should return empty array when subtitles disabled', () => {
302316
const config = { subtitlesEnabled: false };
@@ -523,6 +537,11 @@ describe('YtdlpCommandBuilder', () => {
523537
expect(result).toContain('--fragment-retries');
524538
expect(result).toContain('2');
525539

540+
// Check temp path args (fixes permission errors on Docker setups)
541+
expect(result).toContain('--paths');
542+
const pathsIndex = result.indexOf('--paths');
543+
expect(result[pathsIndex + 1]).toMatch(/^temp:/);
544+
526545
// Check progress template
527546
expect(result).toContain('--newline');
528547
expect(result).toContain('--progress');

server/modules/download/downloadExecutor.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,7 @@ class DownloadExecutor {
412412
env: {
413413
...process.env,
414414
YOUTARR_JOB_ID: jobId,
415-
TMPDIR: '/tmp',
415+
TMPDIR: tempPathManager.getTempBasePath(),
416416
// Pass subfolder override to post-processor (empty string means no override)
417417
...(subfolderOverride !== null && subfolderOverride !== undefined
418418
? { YOUTARR_SUBFOLDER_OVERRIDE: subfolderOverride }

server/modules/download/ytdlpCommandBuilder.js

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,17 @@ class YtdlpCommandBuilder {
125125
return [];
126126
}
127127

128+
/**
129+
* Build temp path args for yt-dlp internal temp files
130+
* This redirects yt-dlp's internal temp files (used during download/merge operations)
131+
* to our already-writable staging directory, fixing permission errors on some Docker setups.
132+
* @returns {string[]} - Array with --paths temp: argument
133+
*/
134+
static buildTempPathArgs() {
135+
const tempPath = tempPathManager.getTempBasePath();
136+
return ['--paths', `temp:${tempPath}`];
137+
}
138+
128139
/**
129140
* Build audio extraction args for MP3 downloads
130141
* @param {string|null} audioFormat - 'video_mp3' or 'mp3_only', null for video only
@@ -150,7 +161,7 @@ class YtdlpCommandBuilder {
150161

151162
/**
152163
* Build arguments that ALWAYS apply to any yt-dlp invocation
153-
* Includes: IPv4 enforcement, proxy, sleep-requests, and cookies
164+
* Includes: IPv4 enforcement, proxy, sleep-requests, cookies, and temp path
154165
* @param {Object} config - Configuration object
155166
* @param {Object} options - Options for building args
156167
* @param {boolean} options.skipSleepRequests - Skip adding --sleep-requests (for single metadata fetches)
@@ -184,6 +195,10 @@ class YtdlpCommandBuilder {
184195
args.push('--cookies', cookiesPath);
185196
}
186197

198+
// Add temp path for yt-dlp's internal temp files
199+
// This fixes permission errors on Docker setups where /tmp may not be writable
200+
args.push(...this.buildTempPathArgs());
201+
187202
return args;
188203
}
189204

server/modules/videoDownloadPostProcessFiles.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ async function downloadChannelThumbnailIfMissing(channelId) {
6161
const result = spawnSync('yt-dlp', ytdlpArgs, {
6262
env: {
6363
...process.env,
64-
TMPDIR: '/tmp'
64+
TMPDIR: tempPathManager.getTempBasePath()
6565
},
6666
encoding: 'utf8'
6767
});

server/modules/ytDlpRunner.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
const { spawn } = require('child_process');
22
const path = require('path');
33
const fs = require('fs');
4+
const tempPathManager = require('./download/tempPathManager');
45

56
class YtDlpRunner {
67
constructor() {
@@ -31,7 +32,7 @@ class YtDlpRunner {
3132
timeout: timeoutMs,
3233
env: {
3334
...process.env,
34-
TMPDIR: '/tmp'
35+
TMPDIR: tempPathManager.getTempBasePath()
3536
}
3637
});
3738

0 commit comments

Comments
 (0)