diff --git a/electron/external-editor.cjs b/electron/external-editor.cjs new file mode 100644 index 000000000..0cc098608 --- /dev/null +++ b/electron/external-editor.cjs @@ -0,0 +1,70 @@ +const path = require("path"); + +// Saved by the file manager after the user picks an editor in a file dialog. +const EXTERNAL_EDITOR_SETTING = "fileManager.externalEditorPath"; + +// Extensions the OS would run rather than open in an editor. With no editor +// configured the temp copy goes to shell.openPath, and its name comes from +// the remote server. +const LAUNCHABLE_EXTENSIONS = new Set([ + // Windows + "application", + "appref-ms", + "bat", + "chm", + "cmd", + "com", + "cpl", + "exe", + "gadget", + "hta", + "inf", + "jar", + "js", + "jse", + "library-ms", + "lnk", + "msc", + "msi", + "msp", + "mst", + "pif", + "ps1", + "psd1", + "psm1", + "py", + "pyw", + "reg", + "scf", + "scr", + "search-ms", + "settingcontent-ms", + "url", + "vbe", + "vbs", + "ws", + "wsf", + "wsh", + "xll", + // macOS + "app", + "command", + "pkg", + "terminal", + "tool", + "workflow", + // Linux + "appimage", + "desktop", + "run", + "sh", +]); + +function isLaunchableFileName(fileName) { + // Windows drops trailing dots and spaces, so "a.exe. " is saved as a.exe. + const name = path.basename(String(fileName || "")).replace(/[. ]+$/, ""); + const extension = path.extname(name).slice(1).toLowerCase(); + return LAUNCHABLE_EXTENSIONS.has(extension); +} + +module.exports = { EXTERNAL_EDITOR_SETTING, isLaunchableFileName }; diff --git a/electron/main.cjs b/electron/main.cjs index 1a20f6bc2..1f6867c8f 100644 --- a/electron/main.cjs +++ b/electron/main.cjs @@ -36,6 +36,10 @@ const { quitApp } = require("./app-quit.cjs"); const { selectLinuxPasswordStore } = require("./linux-password-store.cjs"); const { resolveLocalShell, repairSpawnHelpers } = require("./local-shell.cjs"); const { registerLocalFileHandlers } = require("./local-files.cjs"); +const { + EXTERNAL_EDITOR_SETTING, + isLaunchableFileName, +} = require("./external-editor.cjs"); const { isExternalLoginUrl, isCallbackPort } = require("./external-login.cjs"); const localTerminalSessions = new Map(); @@ -701,6 +705,9 @@ let tray = null; let isQuitting = false; const tempFiles = new Map(); const externalEditorSessions = new Map(); +// Paths the user picked in an open dialog this session. Only these may be +// saved as the external editor, so the renderer cannot name an executable. +const dialogChosenPaths = new Set(); const isDev = process.env.NODE_ENV === "development" || !app.isPackaged; const appRoot = isDev ? process.cwd() : path.join(__dirname, ".."); @@ -3163,18 +3170,16 @@ ipcMain.handle("get-c2s-tunnel-preset-default-name", () => { return `[${date}] ${computerName} (${platform} ${release})`; }); +function readSetting(key) { + const settingsPath = path.join(app.getPath("userData"), "settings.json"); + if (!fs.existsSync(settingsPath)) return null; + const settings = JSON.parse(fs.readFileSync(settingsPath, "utf8")); + return settings[key] !== undefined ? settings[key] : null; +} + ipcMain.handle("get-setting", (event, key) => { try { - const userDataPath = app.getPath("userData"); - const settingsPath = path.join(userDataPath, "settings.json"); - - if (!fs.existsSync(settingsPath)) { - return null; - } - - const settingsData = fs.readFileSync(settingsPath, "utf8"); - const settings = JSON.parse(settingsData); - return settings[key] !== undefined ? settings[key] : null; + return readSetting(key); } catch (error) { console.error("Error reading setting:", error); return null; @@ -3182,6 +3187,16 @@ ipcMain.handle("get-setting", (event, key) => { }); ipcMain.handle("set-setting", (event, key, value) => { + if (typeof key !== "string") { + return { success: false, error: "Invalid setting key" }; + } + if ( + key === EXTERNAL_EDITOR_SETTING && + value && + !dialogChosenPaths.has(value) + ) { + return { success: false, error: "Choose the editor in the file dialog" }; + } try { const userDataPath = app.getPath("userData"); const settingsPath = path.join(userDataPath, "settings.json"); @@ -3205,94 +3220,6 @@ ipcMain.handle("set-setting", (event, key, value) => { } }); -ipcMain.handle("get-session-cookie", async (_event, name, targetUrl) => { - try { - const ses = mainWindow?.webContents?.session; - if (!ses) return null; - const cookies = await ses.cookies.get({ - name, - ...(targetUrl ? { url: targetUrl } : {}), - }); - const cookie = cookies.find((candidate) => - cookieMatchesUrl(candidate, targetUrl), - ); - return ( - cookie?.value || - getRememberedElectronAuthCookie(name, targetUrl)?.value || - null - ); - } catch (error) { - console.error("Failed to get session cookie:", error); - return getRememberedElectronAuthCookie(name, targetUrl)?.value || null; - } -}); - -function cookieMatchesUrl(cookie, targetUrl) { - if (!targetUrl) return true; - - try { - const targetHost = new URL(targetUrl).hostname; - const cookieDomain = (cookie.domain || "").replace(/^\./, ""); - - return ( - cookieDomain === targetHost || - targetHost.endsWith(`.${cookieDomain}`) || - (!cookieDomain && targetHost === "localhost") - ); - } catch { - return true; - } -} - -ipcMain.handle( - "wait-session-cookie", - async (_event, name, targetUrl, previousValue, timeoutMs = 5000) => { - const ses = mainWindow?.webContents?.session; - if (!ses) return { success: false, error: "No Electron session" }; - - const existingCookies = await ses.cookies.get({ - name, - ...(targetUrl ? { url: targetUrl } : {}), - }); - const existingCookie = existingCookies.find((cookie) => - cookieMatchesUrl(cookie, targetUrl), - ); - if (existingCookie?.value && existingCookie.value !== previousValue) { - return { success: true, value: existingCookie.value }; - } - - const rememberedCookie = getRememberedElectronAuthCookie(name, targetUrl); - if (rememberedCookie?.value && rememberedCookie.value !== previousValue) { - return { success: true, value: rememberedCookie.value }; - } - - return new Promise((resolve) => { - const timeout = setTimeout(() => { - ses.cookies.off("changed", onCookieChanged); - resolve({ success: false, error: "Timed out waiting for cookie" }); - }, timeoutMs); - - function onCookieChanged(_event, cookie, _cause, removed) { - if ( - removed || - cookie.name !== name || - !cookie.value || - cookie.value === previousValue || - !cookieMatchesUrl(cookie, targetUrl) - ) { - return; - } - - clearTimeout(timeout); - ses.cookies.off("changed", onCookieChanged); - resolve({ success: true, value: cookie.value }); - } - - ses.cookies.on("changed", onCookieChanged); - }); - }, -); - ipcMain.handle("clear-session-cookies", async () => { try { clearPersistedElectronAuthCookies(); @@ -3420,7 +3347,9 @@ ipcMain.handle("show-save-dialog", async (_event, options) => { }); ipcMain.handle("show-open-dialog", async (_event, options) => { - return dialog.showOpenDialog(mainWindow, options || {}); + const result = await dialog.showOpenDialog(mainWindow, options || {}); + for (const filePath of result.filePaths) dialogChosenPaths.add(filePath); + return result; }); function toLocalFileType(stat) { @@ -3743,6 +3672,15 @@ ipcMain.handle("cleanup-temp-file", (_event, tempId) => { ipcMain.handle("open-external-editor", async (_event, fileData) => { try { + // The editor comes from the saved setting, never from the caller. + const editorPath = readSetting(EXTERNAL_EDITOR_SETTING) || null; + if (!editorPath && isLaunchableFileName(fileData?.fileName)) { + return { + success: false, + error: "Choose an external editor to open this file type", + }; + } + const result = createManagedTempFile( fileData?.fileName, fileData?.content, @@ -3764,10 +3702,6 @@ ipcMain.handle("open-external-editor", async (_event, fileData) => { lastMtimeMs: stat.mtimeMs, }); - const editorPath = - typeof fileData?.editorPath === "string" && fileData.editorPath.trim() - ? fileData.editorPath.trim() - : null; const openError = await openPathWithEditor(result.path, editorPath); if (openError) { closeExternalEditorSession(editId); diff --git a/electron/preload.js b/electron/preload.js index 1586e8996..f08063d8f 100644 --- a/electron/preload.js +++ b/electron/preload.js @@ -87,16 +87,6 @@ contextBridge.exposeInMainWorld("electronAPI", { }, clearSessionCookies: () => ipcRenderer.invoke("clear-session-cookies"), - getSessionCookie: (name, targetUrl) => - ipcRenderer.invoke("get-session-cookie", name, targetUrl), - waitForSessionCookie: (name, targetUrl, previousValue, timeoutMs) => - ipcRenderer.invoke( - "wait-session-cookie", - name, - targetUrl, - previousValue, - timeoutMs, - ), externalBrowserLogin: (authUrl, callbackPort) => ipcRenderer.invoke("external-browser-login", authUrl, callbackPort), diff --git a/scripts/electron-external-editor.test.ts b/scripts/electron-external-editor.test.ts new file mode 100644 index 000000000..dd8862333 --- /dev/null +++ b/scripts/electron-external-editor.test.ts @@ -0,0 +1,40 @@ +import { createRequire } from "node:module"; +import { describe, expect, it } from "vitest"; + +const require = createRequire(import.meta.url); +const { isLaunchableFileName } = require("../electron/external-editor.cjs") as { + isLaunchableFileName: (fileName: unknown) => boolean; +}; + +describe("Electron external editor", () => { + it("refuses remote file names the OS would run instead of open", () => { + for (const name of [ + "payload.exe", + "run.BAT", + "script.ps1", + "a.exe. ", + "a.cmd...", + "../../evil.vbs", + "start.command", + "launch.desktop", + "install.sh", + ]) { + expect(isLaunchableFileName(name)).toBe(true); + } + }); + + it("opens ordinary text files", () => { + for (const name of [ + "nginx.conf", + "README.md", + "Dockerfile", + ".bashrc", + "config.json", + "index.ts", + "", + undefined, + ]) { + expect(isLaunchableFileName(name)).toBe(false); + } + }); +}); diff --git a/scripts/electron-security-boundary.test.ts b/scripts/electron-security-boundary.test.ts index 1d1d0049b..6de78ce7d 100644 --- a/scripts/electron-security-boundary.test.ts +++ b/scripts/electron-security-boundary.test.ts @@ -32,9 +32,62 @@ describe("Electron security boundary", () => { } }); + it("never hands the session cookie to the renderer over IPC", () => { + for (const channel of ["get-session-cookie", "wait-session-cookie"]) { + expect(main).not.toContain(`"${channel}"`); + expect(preload).not.toContain(`"${channel}"`); + } + }); + + it("launches only the editor saved from a file dialog", () => { + expect(main).not.toContain("fileData?.editorPath"); + expect(main).toContain("readSetting(EXTERNAL_EDITOR_SETTING)"); + expect(main).toContain("dialogChosenPaths.has(value)"); + }); + it("scopes proxy headers and basic auth to the linked origin", () => { expect(main).toContain('"sync-proxy-config"'); expect(main).toContain("linkedServer.applyLinkHeaders("); expect(main).toContain("linkedServer.answerLogin("); }); }); + +it("rejects setting keys that coerce to the protected editor key", async () => { + const { runInNewContext } = await import("node:vm"); + const { vi } = await import("vitest"); + const writeFileSync = vi.fn(); + let handler: (event: unknown, key: unknown, value: unknown) => unknown; + runInNewContext( + main.slice( + main.indexOf('ipcMain.handle("set-setting"'), + main.indexOf('ipcMain.handle("clear-session-cookies"'), + ), + { + ipcMain: { + handle: (_: string, callback: typeof handler) => { + handler = callback; + }, + }, + EXTERNAL_EDITOR_SETTING: "fileManager.externalEditorPath", + dialogChosenPaths: new Set(["/chosen/editor"]), + app: { getPath: () => "/settings" }, + path: { join: (...parts: string[]) => parts.join("/") }, + fs: { existsSync: () => false, mkdirSync: vi.fn(), writeFileSync }, + console, + }, + ); + for (const key of [["fileManager.externalEditorPath"], null, {}, 42]) { + expect(handler!({}, key, "/unapproved/editor")).toEqual({ + success: false, + error: "Invalid setting key", + }); + } + expect(writeFileSync).not.toHaveBeenCalled(); + expect( + handler!({}, "fileManager.externalEditorPath", "/unapproved/editor"), + ).toEqual({ success: false, error: "Choose the editor in the file dialog" }); + expect( + handler!({}, "fileManager.externalEditorPath", "/chosen/editor"), + ).toEqual({ success: true }); + expect(writeFileSync).toHaveBeenCalledOnce(); +});