From 5a9f2de3907e55df59b4827cc987d8559ff15394 Mon Sep 17 00:00:00 2001 From: debont80 Date: Mon, 29 Jun 2026 10:15:59 -0400 Subject: [PATCH] Encrypt credentials at rest; unify clipboard-link suggestions Encrypt proxy / youtubePoToken / updateToken on disk via safeStorage (DPAPI on Windows), with a one-time launch migration for legacy plaintext. Decrypted before reaching callers/renderer; backup export still writes clear, as documented. Harden updater token handling to refuse cross-origin redirects on the authenticated REST check. Extract the clipboard-link logic from DownloadBar into the shared useClipboardLink hook: add an optional filter (library skips single-video links), an offer() for external aerofetch:// / .url links, and a source field driving banner wording. Add looksLikeSingleVideo plus its unit tests. Co-Authored-By: Claude Opus 4.8 --- src/main/index.ts | 12 ++- src/main/settings.ts | 87 ++++++++++++++-- src/main/updater.ts | 12 ++- src/renderer/src/components/DownloadBar.tsx | 81 +++------------ src/renderer/src/components/LibraryView.tsx | 8 +- src/renderer/src/components/SettingsView.tsx | 2 +- src/renderer/src/useClipboardLink.ts | 101 +++++++++++++++---- test/clipboardLink.test.ts | 75 ++++++++++++++ 8 files changed, 278 insertions(+), 100 deletions(-) create mode 100644 test/clipboardLink.test.ts diff --git a/src/main/index.ts b/src/main/index.ts index eb4255e..02b4dfe 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -23,7 +23,13 @@ import { hasActiveDownloads } from './download' import { runTerminal, cancelTerminal } from './terminal' -import { getSettings, setSettings, ensureMediaDirs, applyLaunchAtStartup } from './settings' +import { + getSettings, + setSettings, + ensureMediaDirs, + applyLaunchAtStartup, + migrateSecretsAtRest +} from './settings' import { listHistory, addHistory, removeHistory, removeManyHistory, clearHistory } from './history' import { listTemplates, saveTemplate, removeTemplate } from './templates' import { setupPortableData } from './portable' @@ -398,6 +404,10 @@ if (isPrimaryInstance) { // exist from first launch (downloads are routed into them by kind). ensureMediaDirs() + // Encrypt any credential still stored as legacy plaintext (from before at-rest + // encryption), once safeStorage is available post-ready. + migrateSecretsAtRest() + // Sync the Windows "run at sign-in" entry with the persisted setting, so it // reflects the user's choice even if they changed it on another install. applyLaunchAtStartup(getSettings().launchAtStartup) diff --git a/src/main/settings.ts b/src/main/settings.ts index e5ef522..fa67807 100644 --- a/src/main/settings.ts +++ b/src/main/settings.ts @@ -1,4 +1,4 @@ -import { app } from 'electron' +import { app, safeStorage } from 'electron' import { join } from 'path' import { mkdirSync } from 'fs' import Store from 'electron-store' @@ -149,6 +149,72 @@ function getStore(): Store { return store } +// --- Credential encryption at rest ------------------------------------------ +// proxy / youtubePoToken / updateToken can carry secrets (a proxy password, API +// tokens). They're stored encrypted via Electron safeStorage (DPAPI on Windows) +// so a leaked settings.json doesn't expose them. They're still decrypted before +// reaching the renderer and exported in clear by backup — this guards the file at +// rest only. +const SECRET_KEYS = ['proxy', 'youtubePoToken', 'updateToken'] as const + +// Marks a value produced by encryptSecret, so a read can tell ciphertext from a +// legacy plaintext value (written before encryption existed, or while safeStorage +// was unavailable) and migrate it on the next write. +const ENC_PREFIX = 'enc:v1:' + +/** Encrypt a secret for storage. Falls back to plaintext where safeStorage is unavailable. */ +function encryptSecret(plain: string): string { + if (!plain) return '' + try { + if (safeStorage.isEncryptionAvailable()) { + return ENC_PREFIX + safeStorage.encryptString(plain).toString('base64') + } + } catch { + /* fall through — store plaintext, as it was before encryption existed */ + } + return plain +} + +/** Decrypt a stored secret. Legacy plaintext is returned as-is; an undecryptable blob → ''. */ +function decryptSecret(stored: string): string { + if (!stored.startsWith(ENC_PREFIX)) return stored + try { + return safeStorage.decryptString(Buffer.from(stored.slice(ENC_PREFIX.length), 'base64')) + } catch { + // Different user/machine or a corrupt blob — drop it rather than surface + // ciphertext into the UI or onto a yt-dlp command line. + return '' + } +} + +/** A copy of settings with the credential fields decrypted for in-process use. */ +function withDecryptedSecrets(raw: Settings): Settings { + const out = { ...raw } + for (const key of SECRET_KEYS) out[key] = decryptSecret(raw[key] ?? '') + return out +} + +/** + * One-time (per launch) migration: re-store any credential still held as legacy + * plaintext as ciphertext. Lets settings files written before at-rest encryption + * get protected without a write on the hot getSettings() path. No-op when there's + * nothing to migrate or safeStorage is unavailable. + */ +export function migrateSecretsAtRest(): void { + let available = false + try { + available = safeStorage.isEncryptionAvailable() + } catch { + return + } + if (!available) return + const s = getStore() + for (const key of SECRET_KEYS) { + const raw = s.get(key) ?? '' + if (raw && !raw.startsWith(ENC_PREFIX)) s.set(key, encryptSecret(raw)) + } +} + export function getSettings(): Settings { const s = getStore() // getSettings() is on hot paths (buildCommand, notification checks, the system- @@ -170,7 +236,9 @@ export function getSettings(): Settings { if (!(ACCENT_COLORS as readonly string[]).includes(cur.accentColor)) { s.set('accentColor', DEFAULTS.accentColor) } - return s.store + // Hand callers (and, via IPC, the renderer) plaintext credentials — they're + // only encrypted on disk (see withDecryptedSecrets / encryptSecret). + return withDecryptedSecrets(s.store) } /** Shallow structural equality for DownloadOptions (sponsorBlockCategories compared by value). */ @@ -276,17 +344,20 @@ export function setSettings(partial: Partial): Settings { break case 'defaultVideoQuality': case 'defaultAudioQuality': - // proxy may carry plaintext credentials (user:pass@host); they are stored - // and exported by exportBackup as-is — documented, not masked. - case 'proxy': case 'rateLimit': case 'youtubePlayerClient': - case 'youtubePoToken': if (typeof value === 'string') s.set(key, value) break + // Credential-bearing fields — encrypted at rest (see encryptSecret). proxy + // may embed user:pass@host; youtubePoToken is an access token. exportBackup + // still writes them in clear (via the decrypted getSettings), as documented. + case 'proxy': + case 'youtubePoToken': + if (typeof value === 'string') s.set(key, encryptSecret(value)) + break case 'updateToken': - // A Gitea token has no spaces; trim and store as-is (like proxy creds). - if (typeof value === 'string') s.set('updateToken', value.trim()) + // A Gitea token has no spaces; trim before encrypting (like proxy creds). + if (typeof value === 'string') s.set('updateToken', encryptSecret(value.trim())) break } } diff --git a/src/main/updater.ts b/src/main/updater.ts index 75d69aa..c744d0a 100644 --- a/src/main/updater.ts +++ b/src/main/updater.ts @@ -15,8 +15,12 @@ import { * Authorization header for the update host. Empty unless the user has set an * updateToken in Settings — needed when the release repo is private or the Gitea * instance requires sign-in for anonymous access (the default on this instance). - * Only ever sent to the host-pinned UPDATE_HOST (see isTrustedDownloadUrl), so a - * redirect can never leak the token to another origin. + * + * The token must never reach an origin other than the host-pinned UPDATE_HOST. + * Every request that carries it guards against that on its own terms: the REST + * check refuses to follow redirects (`redirect: 'error'`), and the download paths + * use `redirect: 'manual'` and re-validate each hop with isTrustedDownloadUrl. So + * a redirect can't bounce the header to another host on any path. */ function authHeader(): Record { const tok = getSettings().updateToken?.trim() @@ -123,6 +127,10 @@ export async function checkForAppUpdate(): Promise { try { const res = await fetch(RELEASE_API, { headers: { Accept: 'application/json', ...authHeader() }, + // The API lives at an exact pinned URL and answers 200 directly. Refusing + // redirects keeps an authenticated check from ever forwarding the token to + // another origin; a stray redirect fails safe into the catch below. + redirect: 'error', signal: controller.signal }) if (!res.ok) { diff --git a/src/renderer/src/components/DownloadBar.tsx b/src/renderer/src/components/DownloadBar.tsx index 9fea3e1..5932909 100644 --- a/src/renderer/src/components/DownloadBar.tsx +++ b/src/renderer/src/components/DownloadBar.tsx @@ -33,18 +33,7 @@ import { sameVideo } from '../store/queueStats' import { useSettings } from '../store/settings' import { Select } from './Select' import { Hint } from './Hint' - -/** A quick heuristic for "this clipboard text is a link worth offering". */ -function looksLikeUrl(text: string): boolean { - const t = text.trim() - if (!/^https?:\/\//i.test(t)) return false - try { - new URL(t) - return true - } catch { - return false - } -} +import { useClipboardLink, looksLikeUrl } from '../useClipboardLink' /** First http(s) URL in a blob of dropped text (one per line; '#' comments skipped). */ function firstUrl(text: string): string | null { @@ -384,62 +373,22 @@ export function DownloadBar(): React.JSX.Element { // bar's global kind. Lets a playlist mix video and audio downloads. const [itemKinds, setItemKinds] = useState>({}) - // Clipboard auto-detect: when the window gains focus and the clipboard holds a - // fresh link, offer it (without clobbering anything the user is already typing). - // The same banner also surfaces links handed to AeroFetch from outside — the - // aerofetch:// protocol or a "Send to" .url file (see onExternalUrl below). - const [suggestion, setSuggestion] = useState(null) - const [suggestionSource, setSuggestionSource] = useState<'clipboard' | 'external'>('clipboard') - const urlRef = useRef('') - urlRef.current = url - const lastSeen = useRef(null) - - useEffect(() => { - let active = true - async function check(): Promise { - if (!useSettings.getState().clipboardWatch || urlRef.current.trim()) return - let text = '' - try { - text = (await window.api.readClipboard()) ?? '' - } catch { - return - } - if (!active) return - text = text.trim() - if (looksLikeUrl(text) && text !== lastSeen.current) { - setSuggestionSource('clipboard') - setSuggestion(text) - } - } - check() - window.addEventListener('focus', check) - return () => { - active = false - window.removeEventListener('focus', check) - } - }, []) - - // A link handed in from outside always takes priority over whatever the - // clipboard banner was showing — it's a direct request, not a guess. - useEffect( - () => - window.api.onExternalUrl((incomingUrl) => { - setSuggestionSource('external') - setSuggestion(incomingUrl) - }), - [] - ) + // Clipboard auto-detect and links handed to AeroFetch from outside (the + // aerofetch:// protocol or a "Send to" .url file) share one suggestion banner, + // driven by the same hook the library's add-source field uses. An external link + // always takes priority over a clipboard guess — it's a direct request. + const { + suggestion, + source: suggestionSource, + accept: acceptLink, + dismiss: dismissSuggestion, + offer: offerLink + } = useClipboardLink(url) + useEffect(() => window.api.onExternalUrl((incoming) => offerLink(incoming, 'external')), [offerLink]) function acceptSuggestion(): void { - if (!suggestion) return - lastSeen.current = suggestion - onUrlChange(suggestion) - setSuggestion(null) - } - - function dismissSuggestion(): void { - lastSeen.current = suggestion - setSuggestion(null) + const link = acceptLink() + if (link) onUrlChange(link) } const usingFormats = kind === 'video' && info !== null && info.formats.length > 0 diff --git a/src/renderer/src/components/LibraryView.tsx b/src/renderer/src/components/LibraryView.tsx index b72c28a..929b9ee 100644 --- a/src/renderer/src/components/LibraryView.tsx +++ b/src/renderer/src/components/LibraryView.tsx @@ -32,7 +32,7 @@ import { import type { MediaItem, Source } from '@shared/ipc' import { useSources } from '../store/sources' import { useSettings } from '../store/settings' -import { useClipboardLink } from '../useClipboardLink' +import { useClipboardLink, looksLikeSingleVideo } from '../useClipboardLink' import { useDownloads, type DownloadStatus } from '../store/downloads' import { thumbUrl } from '../thumb' import { MediaThumb } from './MediaThumb' @@ -271,9 +271,9 @@ export function LibraryView(): React.JSX.Element { const [url, setUrl] = useState('') const [error, setError] = useState(null) - // Offer a freshly-copied channel/playlist link, the same way the Downloads tab - // offers a copied video link. - const clip = useClipboardLink(url) + // Offer a freshly-copied link the way the Downloads tab does, but skip single + // videos — a library source is a channel/playlist to sync, not a one-off. + const clip = useClipboardLink(url, (u) => !looksLikeSingleVideo(u)) const [selected, setSelected] = useState>(new Set()) const [batchNote, setBatchNote] = useState(null) const [syncNote, setSyncNote] = useState(null) diff --git a/src/renderer/src/components/SettingsView.tsx b/src/renderer/src/components/SettingsView.tsx index cc01dd8..d7bae13 100644 --- a/src/renderer/src/components/SettingsView.tsx +++ b/src/renderer/src/components/SettingsView.tsx @@ -968,7 +968,7 @@ export function SettingsView(): React.JSX.Element { ` with no + * `list=` context), so a channel/playlist link — on YouTube or any other site — is + * never wrongly rejected. A `list=` param means "add the playlist", so it's kept. + */ +export function looksLikeSingleVideo(text: string): boolean { + let u: URL + try { + u = new URL(text.trim()) + } catch { + return false + } + if (u.searchParams.has('list')) return false + const host = u.hostname.replace(/^www\./, '').toLowerCase() + if (host === 'youtube.com' || host === 'm.youtube.com' || host === 'music.youtube.com') { + return u.pathname === '/watch' && u.searchParams.has('v') + } + if (host === 'youtu.be') { + return /^\/[\w-]{6,}$/.test(u.pathname) + } + return false +} + +/** Where an offered link came from — drives the banner's wording. */ +export type SuggestionSource = 'clipboard' | 'external' + interface ClipboardLink { - /** the freshly-copied link being offered, or null */ + /** the link currently being offered, or null */ suggestion: string | null + /** where `suggestion` came from: a clipboard guess vs. a link handed in from outside */ + source: SuggestionSource /** accept the suggestion: clears it and returns the link (or null if none) */ accept: () => string | null /** dismiss the suggestion without using it */ dismiss: () => void + /** + * Offer a link from outside the clipboard — e.g. the aerofetch:// protocol or a + * "Send to" .url file. Always takes priority over a clipboard guess and is shown + * even when clipboard-watch is off (it's a direct request, not a guess) and even + * while the field has text (the banner only fills the field once accepted). + * Stable identity, so it's safe to use as an effect dependency. + */ + offer: (url: string, source?: SuggestionSource) => void } /** @@ -28,14 +67,31 @@ interface ClipboardLink { * can auto-suggest a copied URL. Respects the `clipboardWatch` setting and never * interrupts text the user is already typing — pass the field's current value as * `currentValue` and the watcher stays quiet while it's non-empty. + * + * Pass `filter` to narrow what the clipboard offers — e.g. the library skips + * single-video links so it only suggests channels/playlists. It gates only the + * clipboard guess; links pushed via `offer()` bypass it (and the other guards), + * since they're explicit requests rather than guesses. */ -export function useClipboardLink(currentValue: string): ClipboardLink { +export function useClipboardLink( + currentValue: string, + filter?: (url: string) => boolean +): ClipboardLink { const [suggestion, setSuggestion] = useState(null) + const [source, setSource] = useState('clipboard') const valueRef = useRef('') valueRef.current = currentValue + // Held in a ref so an inline `filter` doesn't have to be memoized to keep the + // focus listener from re-subscribing every render. + const filterRef = useRef(filter) + filterRef.current = filter // The last link we offered or that the user dismissed — so re-focusing doesn't // keep re-offering the same one. const lastSeen = useRef(null) + // Mirror the live suggestion so accept/dismiss can read it without depending on + // it — keeps their identity stable across renders. + const suggestionRef = useRef(null) + suggestionRef.current = suggestion useEffect(() => { let active = true @@ -49,7 +105,11 @@ export function useClipboardLink(currentValue: string): ClipboardLink { } if (!active) return text = text.trim() - if (looksLikeUrl(text) && text !== lastSeen.current) setSuggestion(text) + const wanted = looksLikeUrl(text) && (filterRef.current?.(text) ?? true) + if (wanted && text !== lastSeen.current) { + setSource('clipboard') + setSuggestion(text) + } } check() window.addEventListener('focus', check) @@ -59,18 +119,23 @@ export function useClipboardLink(currentValue: string): ClipboardLink { } }, []) - return { - suggestion, - accept: () => { - if (!suggestion) return null - lastSeen.current = suggestion - const accepted = suggestion - setSuggestion(null) - return accepted - }, - dismiss: () => { - lastSeen.current = suggestion - setSuggestion(null) - } - } + const accept = useCallback((): string | null => { + const link = suggestionRef.current + if (!link) return null + lastSeen.current = link + setSuggestion(null) + return link + }, []) + + const dismiss = useCallback((): void => { + lastSeen.current = suggestionRef.current + setSuggestion(null) + }, []) + + const offer = useCallback((url: string, src: SuggestionSource = 'external'): void => { + setSource(src) + setSuggestion(url) + }, []) + + return { suggestion, source, accept, dismiss, offer } } diff --git a/test/clipboardLink.test.ts b/test/clipboardLink.test.ts new file mode 100644 index 0000000..047e014 --- /dev/null +++ b/test/clipboardLink.test.ts @@ -0,0 +1,75 @@ +import { describe, it, expect } from 'vitest' +import { looksLikeUrl, looksLikeSingleVideo } from '../src/renderer/src/useClipboardLink' + +describe('looksLikeUrl', () => { + it('accepts http(s) URLs', () => { + expect(looksLikeUrl('https://youtube.com/watch?v=abc')).toBe(true) + expect(looksLikeUrl('http://example.com')).toBe(true) + expect(looksLikeUrl('https://example.com/a/b?c=d#frag')).toBe(true) + }) + + it('is scheme-case-insensitive', () => { + expect(looksLikeUrl('HTTPS://example.com')).toBe(true) + expect(looksLikeUrl('HtTp://example.com')).toBe(true) + }) + + it('trims surrounding whitespace', () => { + expect(looksLikeUrl(' https://example.com\n')).toBe(true) + expect(looksLikeUrl('\t http://example.com \t')).toBe(true) + }) + + it('rejects non-http(s) schemes', () => { + expect(looksLikeUrl('ftp://example.com')).toBe(false) + expect(looksLikeUrl('file:///c:/x')).toBe(false) + expect(looksLikeUrl('magnet:?xt=urn:btih:abc')).toBe(false) + expect(looksLikeUrl('javascript:alert(1)')).toBe(false) + expect(looksLikeUrl('mailto:a@b.com')).toBe(false) + }) + + it('rejects bare domains and scheme-less text', () => { + expect(looksLikeUrl('www.example.com')).toBe(false) + expect(looksLikeUrl('example.com/watch')).toBe(false) + expect(looksLikeUrl('see http://example.com')).toBe(false) // must start with the scheme + }) + + it('rejects empty, whitespace, and malformed input', () => { + expect(looksLikeUrl('')).toBe(false) + expect(looksLikeUrl(' ')).toBe(false) + expect(looksLikeUrl('not a link')).toBe(false) + expect(looksLikeUrl('http://')).toBe(false) // passes the prefix test but is not a valid URL + }) +}) + +describe('looksLikeSingleVideo', () => { + it('flags YouTube single-video URLs', () => { + expect(looksLikeSingleVideo('https://www.youtube.com/watch?v=dQw4w9WgXcQ')).toBe(true) + expect(looksLikeSingleVideo('https://youtube.com/watch?v=abc123')).toBe(true) + expect(looksLikeSingleVideo('https://m.youtube.com/watch?v=abc123')).toBe(true) + expect(looksLikeSingleVideo('https://music.youtube.com/watch?v=abc123')).toBe(true) + expect(looksLikeSingleVideo('https://youtu.be/dQw4w9WgXcQ')).toBe(true) + }) + + it('keeps anything with a playlist context (?list=)', () => { + // A video opened in a playlist → the user likely wants the whole playlist. + expect(looksLikeSingleVideo('https://www.youtube.com/watch?v=abc&list=PL123')).toBe(false) + expect(looksLikeSingleVideo('https://youtu.be/abc?list=PL123')).toBe(false) + }) + + it('keeps channels and playlists', () => { + expect(looksLikeSingleVideo('https://www.youtube.com/@SomeChannel')).toBe(false) + expect(looksLikeSingleVideo('https://www.youtube.com/channel/UC123')).toBe(false) + expect(looksLikeSingleVideo('https://www.youtube.com/c/SomeChannel')).toBe(false) + expect(looksLikeSingleVideo('https://www.youtube.com/playlist?list=PL123')).toBe(false) + }) + + it('does not flag single videos on other sites (conservative)', () => { + // We can't reliably classify arbitrary hosts, so never reject them. + expect(looksLikeSingleVideo('https://vimeo.com/123456789')).toBe(false) + expect(looksLikeSingleVideo('https://example.com/watch?v=abc')).toBe(false) + }) + + it('returns false for non-URLs', () => { + expect(looksLikeSingleVideo('not a url')).toBe(false) + expect(looksLikeSingleVideo('')).toBe(false) + }) +})