diff --git a/src/main/updater.ts b/src/main/updater.ts index 63e9e8b..791c49c 100644 --- a/src/main/updater.ts +++ b/src/main/updater.ts @@ -1,7 +1,7 @@ -import { app, shell, type WebContents } from 'electron' -import { createWriteStream } from 'fs' -import { stat } from 'fs/promises' -import { join } from 'path' +import { app, net, shell, type WebContents } from 'electron' +import { createWriteStream, type WriteStream } from 'fs' +import { stat, unlink } from 'fs/promises' +import { join, normalize, dirname } from 'path' import { IpcChannels, type AppUpdateInfo, @@ -24,8 +24,10 @@ const UPDATE_REPO = 'AeroFetch' const RELEASE_API = `${UPDATE_HOST}/api/v1/repos/${UPDATE_OWNER}/${UPDATE_REPO}/releases/latest` // Security: only ever download or execute a file served by the trusted update -// host over HTTPS, even though downloadUrl comes from the release JSON. A -// tampered/man-in-the-middled response can't redirect us to an arbitrary host. +// host over HTTPS. downloadUrl comes from the release JSON, and Gitea 302-redirects +// release downloads to its attachment store — so this is re-checked on EVERY +// redirect hop (see downloadAppUpdate), which is what stops a tampered/MITM'd +// response from bouncing us off the trusted host. function isTrustedDownloadUrl(url: string): boolean { try { const u = new URL(url) @@ -40,11 +42,18 @@ function updateDir(): string { return app.getPath('temp') } -/** Parse a version like 'v1.2.3' / '1.2.3' into comparable numeric components. */ +/** + * Parse 'v1.2.3' / '1.2.3-rc.1' into the comparable numeric components of the + * release CORE — the x.y.z before any '-prerelease' or '+build' suffix. Dropping + * the suffix means a prerelease (e.g. 0.5.0-rc.1) never sorts ABOVE its final + * release (0.5.0): at most it compares equal, so we won't offer a prerelease as + * an "upgrade" over the same shipped version. + */ function parseVersion(v: string): number[] { return v .replace(/^v/i, '') - .split(/[.\-+]/) + .split(/[-+]/)[0] + .split('.') .map((p) => parseInt(p, 10)) .filter((n) => !Number.isNaN(n)) } @@ -82,8 +91,15 @@ function pickInstaller(assets: GiteaAsset[]): GiteaAsset | undefined { /** Query the configured repo's latest release and compare it to the running app. */ export async function checkForAppUpdate(): Promise { const currentVersion = app.getVersion() + // Bound the check so a stalled/unreachable server can't hang the UI spinner + // indefinitely — mirrors the timeouts on the yt-dlp calls. + const controller = new AbortController() + const timer = setTimeout(() => controller.abort(), 15_000) try { - const res = await fetch(RELEASE_API, { headers: { Accept: 'application/json' } }) + const res = await fetch(RELEASE_API, { + headers: { Accept: 'application/json' }, + signal: controller.signal + }) if (!res.ok) { const auth = res.status === 401 || res.status === 403 || res.status === 404 return { @@ -105,76 +121,162 @@ export async function checkForAppUpdate(): Promise { currentVersion, latestVersion, notes: rel.body?.trim() || undefined, - htmlUrl: rel.html_url, + // Only ever hand the OS browser a release page on our own trusted host. + htmlUrl: rel.html_url && isTrustedDownloadUrl(rel.html_url) ? rel.html_url : undefined, downloadUrl: asset?.browser_download_url, assetName: asset?.name } } catch (e) { + const timedOut = e instanceof Error && e.name === 'AbortError' return { ok: false, available: false, currentVersion, - error: e instanceof Error ? e.message : String(e) + error: timedOut + ? 'Update check timed out — the server took too long to respond.' + : e instanceof Error + ? e.message + : String(e) } + } finally { + clearTimeout(timer) } } +/** How long the download may stall (no bytes received) before we give up. */ +const DOWNLOAD_IDLE_TIMEOUT_MS = 60_000 + /** Stream the installer to a temp file, pushing progress events to the renderer. */ export async function downloadAppUpdate(url: string, wc: WebContents): Promise { if (!isTrustedDownloadUrl(url)) { return { ok: false, error: 'Refused to download from an untrusted location.' } } - try { - const res = await fetch(url) - if (!res.ok || !res.body) return { ok: false, error: `Download failed (HTTP ${res.status}).` } - const total = Number(res.headers.get('content-length')) || undefined - // Derive a safe .exe filename from the URL; fall back to a fixed name. - const urlName = decodeURIComponent(new URL(url).pathname.split('/').pop() || '') - const safeName = /^[\w.\- ]+\.exe$/i.test(urlName) ? urlName : 'AeroFetch-Setup.exe' - const filePath = join(updateDir(), safeName) + // Derive a safe .exe filename from the URL; fall back to a fixed name. + const urlName = decodeURIComponent(new URL(url).pathname.split('/').pop() || '') + const safeName = /^[\w.\- ]+\.exe$/i.test(urlName) ? urlName : 'AeroFetch-Setup.exe' + const filePath = join(updateDir(), safeName) - const fileStream = createWriteStream(filePath) - let received = 0 - const reader = res.body.getReader() - for (;;) { - const { done, value } = await reader.read() - if (done) break - received += value.length - // Respect backpressure so a ~300 MB installer doesn't buffer in memory. - if (!fileStream.write(Buffer.from(value))) { - await new Promise((resolve) => fileStream.once('drain', resolve)) + return new Promise((resolve) => { + let settled = false + let fileStream: WriteStream | null = null + let idle: NodeJS.Timeout | null = null + + const finish = (result: AppUpdateDownload): void => { + if (settled) return + settled = true + if (idle) clearTimeout(idle) + if (!result.ok) { + // Close the handle before unlinking (Windows won't delete an open file) + // and never leave a partial/aborted installer lying around in temp. + if (fileStream && !fileStream.destroyed) fileStream.destroy() + unlink(filePath).catch(() => {}) } - const progress: AppUpdateProgress = { - received, - total, - fraction: total ? received / total : undefined - } - if (!wc.isDestroyed()) wc.send(IpcChannels.appUpdateProgress, progress) + resolve(result) } - await new Promise((resolve, reject) => - fileStream.end((err?: Error | null) => (err ? reject(err) : resolve())) - ) - return { ok: true, filePath } - } catch (e) { - return { ok: false, error: e instanceof Error ? e.message : String(e) } - } + + // net.request (not fetch) so we can re-validate the host on EVERY redirect + // hop: Gitea 302s a release download to its attachment store, and undici's + // fetch hides the Location header under redirect:'manual', so it can't gate + // hops. 'manual' here means a hop only proceeds if we call followRedirect(). + const request = net.request({ url, redirect: 'manual' }) + + const armIdle = (): void => { + if (idle) clearTimeout(idle) + idle = setTimeout(() => { + request.abort() + finish({ ok: false, error: 'Download stalled — please try again.' }) + }, DOWNLOAD_IDLE_TIMEOUT_MS) + } + + request.on('redirect', (_status, _method, redirectUrl) => { + if (!isTrustedDownloadUrl(redirectUrl)) { + request.abort() + finish({ ok: false, error: 'Refused to follow a redirect to an untrusted location.' }) + return + } + request.followRedirect() + }) + + request.on('response', (response) => { + const status = response.statusCode + if (status < 200 || status >= 300) { + request.abort() + finish({ ok: false, error: `Download failed (HTTP ${status}).` }) + return + } + + const lenHeader = response.headers['content-length'] + const total = Number(Array.isArray(lenHeader) ? lenHeader[0] : lenHeader) || undefined + + const stream = createWriteStream(filePath) + fileStream = stream + // Electron's IncomingMessage is event-based (no .pipe). pause()/resume() + // exist at runtime but aren't in its type, so feature-detect them to apply + // backpressure — without it a ~300 MB installer can buffer in memory. + const flow = response as unknown as { pause?(): void; resume?(): void } + let received = 0 + armIdle() + + response.on('data', (chunk: Buffer) => { + armIdle() + received += chunk.length + if (!stream.write(chunk) && flow.pause && flow.resume) { + flow.pause() + stream.once('drain', () => flow.resume?.()) + } + if (!wc.isDestroyed()) { + const progress: AppUpdateProgress = { + received, + total, + fraction: total ? received / total : undefined + } + wc.send(IpcChannels.appUpdateProgress, progress) + } + }) + + response.on('end', () => { + stream.end(() => { + // A truncated download (connection dropped mid-stream) must never be run. + if (total !== undefined && received !== total) { + finish({ ok: false, error: 'The download was incomplete — please try again.' }) + return + } + finish({ ok: true, filePath }) + }) + }) + + // A mid-stream abort emits neither 'end' nor always 'error'; catch it so the + // promise can't hang. + response.on('aborted', () => finish({ ok: false, error: 'The download was interrupted.' })) + response.on('error', (e: Error) => finish({ ok: false, error: e.message })) + stream.on('error', (e: Error) => finish({ ok: false, error: e.message })) + }) + + request.on('error', (e: Error) => finish({ ok: false, error: e.message })) + request.end() + }) } +/** Let the launched installer spawn before we quit to release our files (ms). */ +const INSTALLER_HANDOFF_MS = 1500 + /** Launch a freshly-downloaded installer, then quit so it can replace the app. */ export async function runAppUpdate(filePath: string): Promise<{ ok: boolean; error?: string }> { try { - // Defence in depth: only run a .exe we just wrote into our own temp dir, never - // an arbitrary path handed over IPC. - const dir = updateDir().toLowerCase() - if (!filePath.toLowerCase().startsWith(dir) || !/\.exe$/i.test(filePath)) { + // Defence in depth: only run a .exe that sits DIRECTLY in our own temp dir, + // never an arbitrary path handed over IPC. Compare the parent directory by + // equality — startsWith() is defeated by a sibling like `_evil\x.exe`. + const expectedDir = normalize(updateDir()).toLowerCase() + const target = normalize(filePath) + if (dirname(target).toLowerCase() !== expectedDir || !/\.exe$/i.test(target)) { return { ok: false, error: 'Refused to run an unexpected file.' } } - await stat(filePath) // throws if the file is missing - const err = await shell.openPath(filePath) // hand the installer to the OS + await stat(target) // throws if the file is missing + const err = await shell.openPath(target) // hand the installer to the OS if (err) return { ok: false, error: err } // Give the installer a beat to spawn, then quit so it can overwrite our files. - setTimeout(() => app.quit(), 1500) + setTimeout(() => app.quit(), INSTALLER_HANDOFF_MS) return { ok: true } } catch (e) { return { ok: false, error: e instanceof Error ? e.message : String(e) } diff --git a/src/renderer/src/components/SettingsView.tsx b/src/renderer/src/components/SettingsView.tsx index b315262..8f52a91 100644 --- a/src/renderer/src/components/SettingsView.tsx +++ b/src/renderer/src/components/SettingsView.tsx @@ -256,7 +256,11 @@ export function SettingsView(): React.JSX.Element { setAppUpd(null) setAppUpdError(null) try { - setAppUpd(await window.api.checkForAppUpdate()) + const info = await window.api.checkForAppUpdate() + // Funnel a failed check into the single error slot rather than storing a + // second not-ok AppUpdateInfo, so only one error message can ever render. + if (info.ok) setAppUpd(info) + else setAppUpdError(info.error ?? 'Update check failed.') } catch (e) { setAppUpdError(e instanceof Error ? e.message : String(e)) } finally { @@ -883,11 +887,6 @@ export function SettingsView(): React.JSX.Element { You're up to date — v{appUpd.currentVersion} is the latest. )} - {appUpd && !appUpd.ok && ( - - {appUpd.error} - - )} {appUpdError && ( {appUpdError}