mirror of
https://github.com/nimbold/Firelink.git
synced 2026-09-02 14:07:57 +00:00
fix(add-window): harden intake admission and destination safety
- retain valid magnet clipboard handoffs and reject malformed magnet URLs - normalize destination identity and fail closed on deleted queues - redact malformed media headers and add focused regression coverage
This commit is contained in:
@@ -1633,10 +1633,10 @@ fn append_ytdlp_add_header(config: &mut String, header: &str) -> Result<bool, St
|
|||||||
return Ok(false);
|
return Ok(false);
|
||||||
}
|
}
|
||||||
let Some((name, _)) = safe_header.split_once(':') else {
|
let Some((name, _)) = safe_header.split_once(':') else {
|
||||||
return Err(format!("invalid HTTP header: {safe_header}"));
|
return Err("invalid HTTP header".to_string());
|
||||||
};
|
};
|
||||||
if name.trim().is_empty() {
|
if name.trim().is_empty() {
|
||||||
return Err(format!("invalid HTTP header: {safe_header}"));
|
return Err("invalid HTTP header name".to_string());
|
||||||
}
|
}
|
||||||
append_ytdlp_config_option(config, "--add-header", &safe_header);
|
append_ytdlp_config_option(config, "--add-header", &safe_header);
|
||||||
Ok(name.trim().eq_ignore_ascii_case("cookie"))
|
Ok(name.trim().eq_ignore_ascii_case("cookie"))
|
||||||
@@ -13831,10 +13831,11 @@ mod tests {
|
|||||||
#[test]
|
#[test]
|
||||||
fn ytdlp_media_headers_reject_invalid_lines() {
|
fn ytdlp_media_headers_reject_invalid_lines() {
|
||||||
let mut config = String::new();
|
let mut config = String::new();
|
||||||
let error = append_ytdlp_http_headers(&mut config, Some("not a header"), None)
|
let error = append_ytdlp_http_headers(&mut config, Some("Cookie=super-secret-value"), None)
|
||||||
.expect_err("invalid header line should be rejected");
|
.expect_err("invalid header line should be rejected");
|
||||||
|
|
||||||
assert!(error.contains("invalid HTTP header"));
|
assert!(error.contains("invalid HTTP header"));
|
||||||
|
assert!(!error.contains("super-secret-value"));
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||||
import { commitDownloadState, dispatchItem, flushDownloadPersistence, getProxyArgs, getSiteLogin, hasStaleTemporaryMediaEstimate, initializeDownloadPersistence, normalizeCustomProxy, normalizePersistedDownloadProgress, normalizePersistedQueueState, normalizePersistedQueues, useDownloadStore } from './useDownloadStore';
|
import { commitDownloadState, dispatchItem, flushDownloadPersistence, getProxyArgs, getSiteLogin, hasStaleTemporaryMediaEstimate, initializeDownloadPersistence, MAIN_QUEUE_ID, normalizeCustomProxy, normalizePersistedDownloadProgress, normalizePersistedQueueState, normalizePersistedQueues, useDownloadStore } from './useDownloadStore';
|
||||||
import { useDownloadProgressStore } from './downloadProgressStore';
|
import { useDownloadProgressStore } from './downloadProgressStore';
|
||||||
import { useSettingsStore } from './useSettingsStore';
|
import { useSettingsStore } from './useSettingsStore';
|
||||||
import * as ipc from '../ipc';
|
import * as ipc from '../ipc';
|
||||||
@@ -107,6 +107,7 @@ describe('useDownloadStore', () => {
|
|||||||
pendingAddBatchName: '',
|
pendingAddBatchName: '',
|
||||||
pendingAddRequestContexts: {},
|
pendingAddRequestContexts: {},
|
||||||
pendingAddRequestVersion: 0,
|
pendingAddRequestVersion: 0,
|
||||||
|
queues: [{ id: MAIN_QUEUE_ID, name: 'Main Queue', isMain: true }],
|
||||||
});
|
});
|
||||||
useDownloadProgressStore.setState({ progressMap: {}, retainedProgressMap: {}, moveProgressMap: {} });
|
useDownloadProgressStore.setState({ progressMap: {}, retainedProgressMap: {}, moveProgressMap: {} });
|
||||||
});
|
});
|
||||||
@@ -1879,6 +1880,12 @@ describe('useDownloadStore', () => {
|
|||||||
|
|
||||||
|
|
||||||
it('adds to the selected queue without dispatching', async () => {
|
it('adds to the selected queue without dispatching', async () => {
|
||||||
|
useDownloadStore.setState({
|
||||||
|
queues: [
|
||||||
|
{ id: MAIN_QUEUE_ID, name: 'Main Queue', isMain: true },
|
||||||
|
{ id: 'queue-b', name: 'Downloads', isMain: false }
|
||||||
|
]
|
||||||
|
});
|
||||||
await useDownloadStore.getState().addDownload({
|
await useDownloadStore.getState().addDownload({
|
||||||
id: 'queue-1',
|
id: 'queue-1',
|
||||||
url: 'https://example.com/queue.bin',
|
url: 'https://example.com/queue.bin',
|
||||||
@@ -1894,6 +1901,23 @@ describe('useDownloadStore', () => {
|
|||||||
expect(ipc.invokeCommand).not.toHaveBeenCalledWith('enqueue_download', expect.anything());
|
expect(ipc.invokeCommand).not.toHaveBeenCalledWith('enqueue_download', expect.anything());
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('rejects Add-to-Queue admission when the selected queue was deleted', async () => {
|
||||||
|
useDownloadStore.setState({
|
||||||
|
queues: [{ id: MAIN_QUEUE_ID, name: 'Main Queue', isMain: true }]
|
||||||
|
});
|
||||||
|
|
||||||
|
await expect(useDownloadStore.getState().addDownload({
|
||||||
|
id: 'orphaned-queue-row',
|
||||||
|
url: 'https://example.com/orphaned.bin',
|
||||||
|
fileName: 'orphaned.bin',
|
||||||
|
category: 'Other',
|
||||||
|
dateAdded: ''
|
||||||
|
}, { type: 'add-to-queue', queueId: 'deleted-queue' })).rejects.toThrow('Queue no longer exists.');
|
||||||
|
|
||||||
|
expect(useDownloadStore.getState().downloads).toEqual([]);
|
||||||
|
expect(vi.mocked(ipc.invokeCommand)).not.toHaveBeenCalledWith('db_commit_download_state', expect.anything());
|
||||||
|
});
|
||||||
|
|
||||||
it('waits for durable admission before dispatching a start-now download', async () => {
|
it('waits for durable admission before dispatching a start-now download', async () => {
|
||||||
const disposePersistence = initializeDownloadPersistence('main');
|
const disposePersistence = initializeDownloadPersistence('main');
|
||||||
const events: string[] = [];
|
const events: string[] = [];
|
||||||
@@ -2170,6 +2194,12 @@ describe('useDownloadStore', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it('normalizes new Torrent rows before resolving their default destination', async () => {
|
it('normalizes new Torrent rows before resolving their default destination', async () => {
|
||||||
|
useDownloadStore.setState({
|
||||||
|
queues: [
|
||||||
|
{ id: MAIN_QUEUE_ID, name: 'Main Queue', isMain: true },
|
||||||
|
{ id: 'queue-torrents', name: 'Torrents', isMain: false }
|
||||||
|
]
|
||||||
|
});
|
||||||
await useDownloadStore.getState().addDownload({
|
await useDownloadStore.getState().addDownload({
|
||||||
id: 'torrent-default',
|
id: 'torrent-default',
|
||||||
url: 'magnet:?xt=urn:btih:default',
|
url: 'magnet:?xt=urn:btih:default',
|
||||||
@@ -2187,6 +2217,12 @@ describe('useDownloadStore', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it('inserts a newly staged queue item before paused rows', async () => {
|
it('inserts a newly staged queue item before paused rows', async () => {
|
||||||
|
useDownloadStore.setState({
|
||||||
|
queues: [
|
||||||
|
{ id: MAIN_QUEUE_ID, name: 'Main Queue', isMain: true },
|
||||||
|
{ id: 'queue-b', name: 'Downloads', isMain: false }
|
||||||
|
]
|
||||||
|
});
|
||||||
useDownloadStore.setState({
|
useDownloadStore.setState({
|
||||||
downloads: [{
|
downloads: [{
|
||||||
id: 'already-paused',
|
id: 'already-paused',
|
||||||
@@ -2216,6 +2252,12 @@ describe('useDownloadStore', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it('carries a media format estimate into numeric progress state', async () => {
|
it('carries a media format estimate into numeric progress state', async () => {
|
||||||
|
useDownloadStore.setState({
|
||||||
|
queues: [
|
||||||
|
{ id: MAIN_QUEUE_ID, name: 'Main Queue', isMain: true },
|
||||||
|
{ id: 'queue-b', name: 'Downloads', isMain: false }
|
||||||
|
]
|
||||||
|
});
|
||||||
await useDownloadStore.getState().addDownload({
|
await useDownloadStore.getState().addDownload({
|
||||||
id: 'media-estimate',
|
id: 'media-estimate',
|
||||||
url: 'https://youtube.com/watch?v=estimate',
|
url: 'https://youtube.com/watch?v=estimate',
|
||||||
|
|||||||
@@ -1834,6 +1834,11 @@ export const useDownloadStore = create<DownloadState>((set, get) => {
|
|||||||
},
|
},
|
||||||
setSelectedPropertiesDownloadId: (id) => set({ selectedPropertiesDownloadId: id }),
|
setSelectedPropertiesDownloadId: (id) => set({ selectedPropertiesDownloadId: id }),
|
||||||
addDownload: async (item, action) => {
|
addDownload: async (item, action) => {
|
||||||
|
if (action.type === 'add-to-queue' && !get().queues.some(queue => queue.id === action.queueId)) {
|
||||||
|
// The Add window can outlive a queue deletion in another view. Never
|
||||||
|
// persist an orphaned staged row under a queue ID that no longer exists.
|
||||||
|
throw new Error('Queue no longer exists.');
|
||||||
|
}
|
||||||
const settings = useSettingsStore.getState();
|
const settings = useSettingsStore.getState();
|
||||||
const normalizedItem = {
|
const normalizedItem = {
|
||||||
...item,
|
...item,
|
||||||
|
|||||||
@@ -13,16 +13,23 @@ describe('clipboard URL extraction', () => {
|
|||||||
|
|
||||||
it('reads only supported, unique download URLs from clipboard text', async () => {
|
it('reads only supported, unique download URLs from clipboard text', async () => {
|
||||||
vi.mocked(readText).mockResolvedValue(
|
vi.mocked(readText).mockResolvedValue(
|
||||||
'https://example.com/file.zip\nhttps://example.com/file.zip ftp://example.com/file.bin sftp://example.com/file.iso mailto:user@example.com'
|
'https://example.com/file.zip\nhttps://example.com/file.zip ftp://example.com/file.bin sftp://example.com/file.iso magnet:?xt=urn:btih:0123456789abcdef0123456789abcdef01234567 mailto:user@example.com'
|
||||||
);
|
);
|
||||||
|
|
||||||
await expect(readClipboardDownloadUrls()).resolves.toEqual([
|
await expect(readClipboardDownloadUrls()).resolves.toEqual([
|
||||||
'https://example.com/file.zip',
|
'https://example.com/file.zip',
|
||||||
'ftp://example.com/file.bin',
|
'ftp://example.com/file.bin',
|
||||||
'sftp://example.com/file.iso',
|
'sftp://example.com/file.iso',
|
||||||
|
'magnet:?xt=urn:btih:0123456789abcdef0123456789abcdef01234567',
|
||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('ignores malformed magnet URLs at the clipboard boundary', async () => {
|
||||||
|
vi.mocked(readText).mockResolvedValue('magnet: magnet:?invalid magnet://tracker/?xt=urn:btih:0123456789abcdef0123456789abcdef01234567');
|
||||||
|
|
||||||
|
await expect(readClipboardDownloadUrls()).resolves.toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
it('preserves clipboard read failures for the caller to handle', async () => {
|
it('preserves clipboard read failures for the caller to handle', async () => {
|
||||||
const error = new Error('clipboard unavailable');
|
const error = new Error('clipboard unavailable');
|
||||||
vi.mocked(readText).mockRejectedValue(error);
|
vi.mocked(readText).mockRejectedValue(error);
|
||||||
|
|||||||
@@ -29,6 +29,12 @@ describe('download locations', () => {
|
|||||||
expect(downloadLocationEquals('/home/Test', 'Movie.MP4', '/home/test', 'movie.mp4', 'linux')).toBe(false);
|
expect(downloadLocationEquals('/home/Test', 'Movie.MP4', '/home/test', 'movie.mp4', 'linux')).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('matches destinations with redundant separators without changing platform case rules', () => {
|
||||||
|
expect(downloadLocationEquals('/Users/test//Downloads/', 'file.zip', '/Users/test/Downloads', 'file.zip', 'macos')).toBe(true);
|
||||||
|
expect(downloadLocationEquals('//Users/test/Downloads', 'file.zip', '/Users/test/Downloads', 'file.zip', 'macos')).toBe(true);
|
||||||
|
expect(downloadLocationEquals('\\\\server\\share\\downloads', 'file.zip', '//server//share/downloads/', 'file.zip', 'windows')).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
it('uses a remembered Add-window directory only when the setting is enabled', () => {
|
it('uses a remembered Add-window directory only when the setting is enabled', () => {
|
||||||
expect(resolveInitialAddWindowLocation(
|
expect(resolveInitialAddWindowLocation(
|
||||||
'D:\\Downloads',
|
'D:\\Downloads',
|
||||||
|
|||||||
@@ -307,7 +307,14 @@ export const downloadLocationEquals = (
|
|||||||
os: string
|
os: string
|
||||||
): boolean => {
|
): boolean => {
|
||||||
const normalize = (value: string) => {
|
const normalize = (value: string) => {
|
||||||
const normalized = value.replace(/\\/g, '/').replace(/\/+$/, '');
|
const slashPath = value.replace(/\\/g, '/');
|
||||||
|
// Collapse redundant separators without destroying a Windows UNC prefix.
|
||||||
|
// Destination strings can come from legacy settings as well as the folder
|
||||||
|
// picker, so lexical equality must not miss the same filesystem target.
|
||||||
|
const leadingSeparators = slashPath.match(/^\/+/);
|
||||||
|
const leadingCount = leadingSeparators ? leadingSeparators[0].length : 0;
|
||||||
|
const prefix = os === 'windows' && leadingCount >= 2 ? '//' : leadingCount > 0 ? '/' : '';
|
||||||
|
const normalized = `${prefix}${slashPath.slice(leadingCount).replace(/\/{2,}/g, '/')}`.replace(/\/+$/, '');
|
||||||
return os === 'windows'
|
return os === 'windows'
|
||||||
? normalized.toLocaleLowerCase()
|
? normalized.toLocaleLowerCase()
|
||||||
: normalized;
|
: normalized;
|
||||||
|
|||||||
+9
-1
@@ -11,7 +11,15 @@ export function extractValidDownloadUrls(text: string): string[] {
|
|||||||
for (const part of parts) {
|
for (const part of parts) {
|
||||||
try {
|
try {
|
||||||
const url = new URL(part);
|
const url = new URL(part);
|
||||||
if (url.protocol === 'http:' || url.protocol === 'https:' || url.protocol === 'ftp:' || url.protocol === 'sftp:') {
|
const isValidMagnet = url.protocol !== 'magnet:' || (
|
||||||
|
!url.username
|
||||||
|
&& !url.password
|
||||||
|
&& !url.hostname
|
||||||
|
&& !url.port
|
||||||
|
&& !url.hash
|
||||||
|
&& url.searchParams.getAll('xt').some(value => /^urn:btih:(?:[0-9a-f]{40}|[a-z2-7]{32})$/i.test(value))
|
||||||
|
);
|
||||||
|
if ((url.protocol === 'http:' || url.protocol === 'https:' || url.protocol === 'ftp:' || url.protocol === 'sftp:' || url.protocol === 'magnet:') && isValidMagnet) {
|
||||||
urls.push(url.toString());
|
urls.push(url.toString());
|
||||||
}
|
}
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
|
|||||||
Reference in New Issue
Block a user