chore: rclone initialization version check (#1683)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Improvements**
* Enforces a minimum RClone version (1.70.0) with clearer startup/log
messages for missing, too-old, or unparseable versions.
* Adjusted initialization timing to a later bootstrap phase for more
reliable startup.

* **Tests**
* Expanded and hardened tests: broader API endpoint coverage, enhanced
HTTP error scenarios, refined request assertions, and comprehensive
RClone version-detection tests (newer/older, missing, malformed,
beta/RC).

* **Chores**
* Simplified permissions configuration by replacing detailed rules with
an empty permissions object and removing a top-level flag.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
This commit is contained in:
Eli Bosley
2025-09-10 11:00:59 -04:00
committed by GitHub
parent 50ea2a3ffb
commit c569043ab5
3 changed files with 185 additions and 169 deletions

View File

@@ -1,123 +1,3 @@
{
"permissions": {
"allow": [
"# Development Commands",
"Bash(pnpm install)",
"Bash(pnpm dev)",
"Bash(pnpm build)",
"Bash(pnpm test)",
"Bash(pnpm test:*)",
"Bash(pnpm lint)",
"Bash(pnpm lint:fix)",
"Bash(pnpm type-check)",
"Bash(pnpm codegen)",
"Bash(pnpm storybook)",
"Bash(pnpm --filter * dev)",
"Bash(pnpm --filter * build)",
"Bash(pnpm --filter * test)",
"Bash(pnpm --filter * lint)",
"Bash(pnpm --filter * codegen)",
"# Git Commands (read-only)",
"Bash(git status)",
"Bash(git diff)",
"Bash(git log)",
"Bash(git branch)",
"Bash(git remote -v)",
"# Search Commands",
"Bash(rg *)",
"# File System (read-only)",
"Bash(ls)",
"Bash(ls -la)",
"Bash(pwd)",
"Bash(find . -name)",
"Bash(find . -type)",
"# Node/NPM Commands",
"Bash(node --version)",
"Bash(pnpm --version)",
"Bash(npx --version)",
"# Environment Commands",
"Bash(echo $*)",
"Bash(which *)",
"# Process Commands",
"Bash(ps aux | grep)",
"Bash(lsof -i)",
"# Documentation Domains",
"WebFetch(domain:tailwindcss.com)",
"WebFetch(domain:github.com)",
"WebFetch(domain:reka-ui.com)",
"WebFetch(domain:nodejs.org)",
"WebFetch(domain:pnpm.io)",
"WebFetch(domain:vitejs.dev)",
"WebFetch(domain:nuxt.com)",
"WebFetch(domain:nestjs.com)",
"# IDE Integration",
"mcp__ide__getDiagnostics",
"# Browser MCP (for testing)",
"mcp__browsermcp__browser_navigate",
"mcp__browsermcp__browser_click",
"mcp__browsermcp__browser_screenshot"
],
"deny": [
"# Dangerous Commands",
"Bash(rm -rf)",
"Bash(chmod 777)",
"Bash(curl)",
"Bash(wget)",
"Bash(ssh)",
"Bash(scp)",
"Bash(sudo)",
"Bash(su)",
"Bash(pkill)",
"Bash(kill)",
"Bash(killall)",
"Bash(python)",
"Bash(python3)",
"Bash(pip)",
"Bash(npm)",
"Bash(yarn)",
"Bash(apt)",
"Bash(brew)",
"Bash(systemctl)",
"Bash(service)",
"Bash(docker)",
"Bash(docker-compose)",
"# File Modification (use Edit/Write tools instead)",
"Bash(sed)",
"Bash(awk)",
"Bash(perl)",
"Bash(echo > *)",
"Bash(echo >> *)",
"Bash(cat > *)",
"Bash(cat >> *)",
"Bash(tee)",
"# Git Write Commands (require explicit user action)",
"Bash(git add)",
"Bash(git commit)",
"Bash(git push)",
"Bash(git pull)",
"Bash(git merge)",
"Bash(git rebase)",
"Bash(git checkout)",
"Bash(git reset)",
"Bash(git clean)",
"# Package Management Write Commands",
"Bash(pnpm add)",
"Bash(pnpm remove)",
"Bash(pnpm update)",
"Bash(pnpm upgrade)"
]
},
"enableAllProjectMcpServers": false
"permissions": {}
}

View File

@@ -12,7 +12,22 @@ import {
UpdateRCloneRemoteDto,
} from '@app/unraid-api/graph/resolvers/rclone/rclone.model.js';
vi.mock('got');
vi.mock('got', () => {
const mockPost = vi.fn();
const gotMock = {
post: mockPost,
};
return {
default: gotMock,
HTTPError: class HTTPError extends Error {
response?: any;
constructor(response?: any) {
super('HTTP Error');
this.response = response;
}
},
};
});
vi.mock('execa');
vi.mock('p-retry');
vi.mock('node:fs', () => ({
@@ -60,7 +75,7 @@ vi.mock('@nestjs/common', async (importOriginal) => {
describe('RCloneApiService', () => {
let service: RCloneApiService;
let mockGot: any;
let mockGotPost: any;
let mockExeca: any;
let mockPRetry: any;
let mockExistsSync: any;
@@ -68,19 +83,19 @@ describe('RCloneApiService', () => {
beforeEach(async () => {
vi.clearAllMocks();
const { default: got } = await import('got');
const got = await import('got');
const { execa } = await import('execa');
const pRetry = await import('p-retry');
const { existsSync } = await import('node:fs');
const { fileExists } = await import('@app/core/utils/files/file-exists.js');
mockGot = vi.mocked(got);
mockGotPost = vi.mocked(got.default.post);
mockExeca = vi.mocked(execa);
mockPRetry = vi.mocked(pRetry.default);
mockExistsSync = vi.mocked(existsSync);
// Mock successful RClone API response for socket check
mockGot.post = vi.fn().mockResolvedValue({ body: { pid: 12345 } });
mockGotPost.mockResolvedValue({ body: { pid: 12345 } });
// Mock RClone binary exists check
vi.mocked(fileExists).mockResolvedValue(true);
@@ -97,10 +112,10 @@ describe('RCloneApiService', () => {
mockPRetry.mockResolvedValue(undefined);
service = new RCloneApiService();
await service.onModuleInit();
await service.onApplicationBootstrap();
// Reset the mock after initialization to prepare for test-specific responses
mockGot.post.mockClear();
mockGotPost.mockClear();
});
describe('getProviders', () => {
@@ -109,15 +124,15 @@ describe('RCloneApiService', () => {
{ name: 'aws', prefix: 's3', description: 'Amazon S3' },
{ name: 'google', prefix: 'drive', description: 'Google Drive' },
];
mockGot.post.mockResolvedValue({
mockGotPost.mockResolvedValue({
body: { providers: mockProviders },
});
const result = await service.getProviders();
expect(result).toEqual(mockProviders);
expect(mockGot.post).toHaveBeenCalledWith(
'http://unix:/tmp/rclone.sock:/config/providers',
expect(mockGotPost).toHaveBeenCalledWith(
expect.stringMatching(/\/config\/providers$/),
expect.objectContaining({
json: {},
responseType: 'json',
@@ -130,7 +145,7 @@ describe('RCloneApiService', () => {
});
it('should return empty array when no providers', async () => {
mockGot.post.mockResolvedValue({ body: {} });
mockGotPost.mockResolvedValue({ body: {} });
const result = await service.getProviders();
@@ -141,15 +156,15 @@ describe('RCloneApiService', () => {
describe('listRemotes', () => {
it('should return list of remotes', async () => {
const mockRemotes = ['backup-s3', 'drive-storage'];
mockGot.post.mockResolvedValue({
mockGotPost.mockResolvedValue({
body: { remotes: mockRemotes },
});
const result = await service.listRemotes();
expect(result).toEqual(mockRemotes);
expect(mockGot.post).toHaveBeenCalledWith(
'http://unix:/tmp/rclone.sock:/config/listremotes',
expect(mockGotPost).toHaveBeenCalledWith(
expect.stringMatching(/\/config\/listremotes$/),
expect.objectContaining({
json: {},
responseType: 'json',
@@ -162,7 +177,7 @@ describe('RCloneApiService', () => {
});
it('should return empty array when no remotes', async () => {
mockGot.post.mockResolvedValue({ body: {} });
mockGotPost.mockResolvedValue({ body: {} });
const result = await service.listRemotes();
@@ -174,13 +189,13 @@ describe('RCloneApiService', () => {
it('should return remote details', async () => {
const input: GetRCloneRemoteDetailsDto = { name: 'test-remote' };
const mockConfig = { type: 's3', provider: 'AWS' };
mockGot.post.mockResolvedValue({ body: mockConfig });
mockGotPost.mockResolvedValue({ body: mockConfig });
const result = await service.getRemoteDetails(input);
expect(result).toEqual(mockConfig);
expect(mockGot.post).toHaveBeenCalledWith(
'http://unix:/tmp/rclone.sock:/config/get',
expect(mockGotPost).toHaveBeenCalledWith(
expect.stringMatching(/\/config\/get$/),
expect.objectContaining({
json: { name: 'test-remote' },
responseType: 'json',
@@ -197,7 +212,7 @@ describe('RCloneApiService', () => {
it('should return remote configuration', async () => {
const input: GetRCloneRemoteConfigDto = { name: 'test-remote' };
const mockConfig = { type: 's3', access_key_id: 'AKIA...' };
mockGot.post.mockResolvedValue({ body: mockConfig });
mockGotPost.mockResolvedValue({ body: mockConfig });
const result = await service.getRemoteConfig(input);
@@ -213,13 +228,13 @@ describe('RCloneApiService', () => {
parameters: { access_key_id: 'AKIA...', secret_access_key: 'secret' },
};
const mockResponse = { success: true };
mockGot.post.mockResolvedValue({ body: mockResponse });
mockGotPost.mockResolvedValue({ body: mockResponse });
const result = await service.createRemote(input);
expect(result).toEqual(mockResponse);
expect(mockGot.post).toHaveBeenCalledWith(
'http://unix:/tmp/rclone.sock:/config/create',
expect(mockGotPost).toHaveBeenCalledWith(
expect.stringMatching(/\/config\/create$/),
expect.objectContaining({
json: {
name: 'new-remote',
@@ -243,13 +258,13 @@ describe('RCloneApiService', () => {
parameters: { access_key_id: 'NEW_AKIA...' },
};
const mockResponse = { success: true };
mockGot.post.mockResolvedValue({ body: mockResponse });
mockGotPost.mockResolvedValue({ body: mockResponse });
const result = await service.updateRemote(input);
expect(result).toEqual(mockResponse);
expect(mockGot.post).toHaveBeenCalledWith(
'http://unix:/tmp/rclone.sock:/config/update',
expect(mockGotPost).toHaveBeenCalledWith(
expect.stringMatching(/\/config\/update$/),
expect.objectContaining({
json: {
name: 'existing-remote',
@@ -269,13 +284,13 @@ describe('RCloneApiService', () => {
it('should delete a remote', async () => {
const input: DeleteRCloneRemoteDto = { name: 'remote-to-delete' };
const mockResponse = { success: true };
mockGot.post.mockResolvedValue({ body: mockResponse });
mockGotPost.mockResolvedValue({ body: mockResponse });
const result = await service.deleteRemote(input);
expect(result).toEqual(mockResponse);
expect(mockGot.post).toHaveBeenCalledWith(
'http://unix:/tmp/rclone.sock:/config/delete',
expect(mockGotPost).toHaveBeenCalledWith(
expect.stringMatching(/\/config\/delete$/),
expect.objectContaining({
json: { name: 'remote-to-delete' },
responseType: 'json',
@@ -296,13 +311,13 @@ describe('RCloneApiService', () => {
options: { delete_on: 'dst' },
};
const mockResponse = { jobid: 'job-123' };
mockGot.post.mockResolvedValue({ body: mockResponse });
mockGotPost.mockResolvedValue({ body: mockResponse });
const result = await service.startBackup(input);
expect(result).toEqual(mockResponse);
expect(mockGot.post).toHaveBeenCalledWith(
'http://unix:/tmp/rclone.sock:/sync/copy',
expect(mockGotPost).toHaveBeenCalledWith(
expect.stringMatching(/\/sync\/copy$/),
expect.objectContaining({
json: {
srcFs: '/source/path',
@@ -323,13 +338,13 @@ describe('RCloneApiService', () => {
it('should return job status', async () => {
const input: GetRCloneJobStatusDto = { jobId: 'job-123' };
const mockStatus = { status: 'running', progress: 0.5 };
mockGot.post.mockResolvedValue({ body: mockStatus });
mockGotPost.mockResolvedValue({ body: mockStatus });
const result = await service.getJobStatus(input);
expect(result).toEqual(mockStatus);
expect(mockGot.post).toHaveBeenCalledWith(
'http://unix:/tmp/rclone.sock:/job/status',
expect(mockGotPost).toHaveBeenCalledWith(
expect.stringMatching(/\/job\/status$/),
expect.objectContaining({
json: { jobid: 'job-123' },
responseType: 'json',
@@ -348,13 +363,13 @@ describe('RCloneApiService', () => {
{ id: 'job-1', status: 'running' },
{ id: 'job-2', status: 'finished' },
];
mockGot.post.mockResolvedValue({ body: mockJobs });
mockGotPost.mockResolvedValue({ body: mockJobs });
const result = await service.listRunningJobs();
expect(result).toEqual(mockJobs);
expect(mockGot.post).toHaveBeenCalledWith(
'http://unix:/tmp/rclone.sock:/job/list',
expect(mockGotPost).toHaveBeenCalledWith(
expect.stringMatching(/\/job\/list$/),
expect.objectContaining({
json: {},
responseType: 'json',
@@ -378,7 +393,7 @@ describe('RCloneApiService', () => {
},
};
Object.setPrototypeOf(httpError, HTTPError.prototype);
mockGot.post.mockRejectedValue(httpError);
mockGotPost.mockRejectedValue(httpError);
await expect(service.getProviders()).rejects.toThrow(
'Rclone API Error (config/providers, HTTP 500): Rclone Error: Internal server error'
@@ -395,7 +410,7 @@ describe('RCloneApiService', () => {
},
};
Object.setPrototypeOf(httpError, HTTPError.prototype);
mockGot.post.mockRejectedValue(httpError);
mockGotPost.mockRejectedValue(httpError);
await expect(service.getProviders()).rejects.toThrow(
'Rclone API Error (config/providers, HTTP 404): Failed to process error response body. Raw body:'
@@ -412,7 +427,7 @@ describe('RCloneApiService', () => {
},
};
Object.setPrototypeOf(httpError, HTTPError.prototype);
mockGot.post.mockRejectedValue(httpError);
mockGotPost.mockRejectedValue(httpError);
await expect(service.getProviders()).rejects.toThrow(
'Rclone API Error (config/providers, HTTP 400): Failed to process error response body. Raw body: invalid json'
@@ -421,17 +436,108 @@ describe('RCloneApiService', () => {
it('should handle non-HTTP errors', async () => {
const networkError = new Error('Network connection failed');
mockGot.post.mockRejectedValue(networkError);
mockGotPost.mockRejectedValue(networkError);
await expect(service.getProviders()).rejects.toThrow('Network connection failed');
});
it('should handle unknown errors', async () => {
mockGot.post.mockRejectedValue('unknown error');
mockGotPost.mockRejectedValue('unknown error');
await expect(service.getProviders()).rejects.toThrow(
'Unknown error calling RClone API (config/providers) with params {}: unknown error'
);
});
});
describe('checkRcloneBinaryExists', () => {
beforeEach(() => {
// Create a new service instance without initializing for these tests
service = new RCloneApiService();
});
it('should return true when rclone version is 1.70.0', async () => {
mockExeca.mockResolvedValueOnce({
stdout: 'rclone v1.70.0\n- os/version: darwin 14.0 (64 bit)\n- os/kernel: 23.0.0 (arm64)',
stderr: '',
} as any);
const result = await (service as any).checkRcloneBinaryExists();
expect(result).toBe(true);
});
it('should return true when rclone version is newer than 1.70.0', async () => {
mockExeca.mockResolvedValueOnce({
stdout: 'rclone v1.75.2\n- os/version: darwin 14.0 (64 bit)\n- os/kernel: 23.0.0 (arm64)',
stderr: '',
} as any);
const result = await (service as any).checkRcloneBinaryExists();
expect(result).toBe(true);
});
it('should return false when rclone version is older than 1.70.0', async () => {
mockExeca.mockResolvedValueOnce({
stdout: 'rclone v1.69.0\n- os/version: darwin 14.0 (64 bit)\n- os/kernel: 23.0.0 (arm64)',
stderr: '',
} as any);
const result = await (service as any).checkRcloneBinaryExists();
expect(result).toBe(false);
});
it('should return false when rclone version is much older', async () => {
mockExeca.mockResolvedValueOnce({
stdout: 'rclone v1.50.0\n- os/version: darwin 14.0 (64 bit)\n- os/kernel: 23.0.0 (arm64)',
stderr: '',
} as any);
const result = await (service as any).checkRcloneBinaryExists();
expect(result).toBe(false);
});
it('should return false when version cannot be parsed', async () => {
mockExeca.mockResolvedValueOnce({
stdout: 'rclone unknown version format',
stderr: '',
} as any);
const result = await (service as any).checkRcloneBinaryExists();
expect(result).toBe(false);
});
it('should return false when rclone binary is not found', async () => {
const error = new Error('Command not found') as any;
error.code = 'ENOENT';
mockExeca.mockRejectedValueOnce(error);
const result = await (service as any).checkRcloneBinaryExists();
expect(result).toBe(false);
});
it('should return false and log error for other exceptions', async () => {
mockExeca.mockRejectedValueOnce(new Error('Some other error'));
const result = await (service as any).checkRcloneBinaryExists();
expect(result).toBe(false);
});
it('should handle beta/rc versions correctly', async () => {
mockExeca.mockResolvedValueOnce({
stdout: 'rclone v1.70.0-beta.1\n- os/version: darwin 14.0 (64 bit)\n- os/kernel: 23.0.0 (arm64)',
stderr: '',
} as any);
const result = await (service as any).checkRcloneBinaryExists();
expect(result).toBe(true);
});
});
});

View File

@@ -1,4 +1,4 @@
import { Injectable, Logger, OnModuleDestroy, OnModuleInit } from '@nestjs/common';
import { Injectable, Logger, OnApplicationBootstrap, OnModuleDestroy } from '@nestjs/common';
import crypto from 'crypto';
import { ChildProcess } from 'node:child_process';
import { mkdir, rm, writeFile } from 'node:fs/promises';
@@ -7,6 +7,7 @@ import { dirname, join } from 'node:path';
import { execa } from 'execa';
import got, { HTTPError } from 'got';
import pRetry from 'p-retry';
import semver from 'semver';
import { sanitizeParams } from '@app/core/log.js';
import { fileExists } from '@app/core/utils/files/file-exists.js';
@@ -25,7 +26,7 @@ import {
import { validateObject } from '@app/unraid-api/graph/resolvers/validation.utils.js';
@Injectable()
export class RCloneApiService implements OnModuleInit, OnModuleDestroy {
export class RCloneApiService implements OnApplicationBootstrap, OnModuleDestroy {
private isInitialized: boolean = false;
private readonly logger = new Logger(RCloneApiService.name);
private rcloneSocketPath: string = '';
@@ -44,7 +45,7 @@ export class RCloneApiService implements OnModuleInit, OnModuleDestroy {
return this.isInitialized;
}
async onModuleInit(): Promise<void> {
async onApplicationBootstrap(): Promise<void> {
// RClone startup disabled - early return
if (ENVIRONMENT === 'production') {
this.logger.debug('RClone startup is disabled');
@@ -239,12 +240,41 @@ export class RCloneApiService implements OnModuleInit, OnModuleDestroy {
}
/**
* Checks if the RClone binary is available on the system
* Checks if the RClone binary is available on the system and meets minimum version requirements
*/
private async checkRcloneBinaryExists(): Promise<boolean> {
try {
await execa('rclone', ['version']);
this.logger.debug('RClone binary is available on the system.');
const result = await execa('rclone', ['version']);
const versionOutput = result.stdout.trim();
// Extract raw version string (format: "rclone vX.XX.X" or "rclone vX.XX.X-beta.X")
const versionMatch = versionOutput.match(/rclone v([\d.\-\w]+)/);
if (!versionMatch) {
this.logger.error('Unable to parse RClone version from output');
return false;
}
const rawVersion = versionMatch[1];
// Use semver.coerce to get base semver from prerelease versions
const coercedVersion = semver.coerce(rawVersion);
if (!coercedVersion) {
this.logger.error(`Failed to parse RClone version: raw="${rawVersion}"`);
return false;
}
const minimumVersion = '1.70.0';
if (!semver.gte(coercedVersion, minimumVersion)) {
this.logger.error(
`RClone version ${rawVersion} (coerced: ${coercedVersion}) is too old. Minimum required version is ${minimumVersion}`
);
return false;
}
this.logger.debug(
`RClone binary is available on the system (version ${rawVersion}, coerced: ${coercedVersion}).`
);
return true;
} catch (error: unknown) {
if (error instanceof Error && 'code' in error && error.code === 'ENOENT') {