Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions apps/desktop/src/main/ipc/sftpIpc.security.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import path from "node:path";
import { describe, expect, it } from "vitest";

import { resolveSafeDragOutPath } from "./sftpIpc";

describe("resolveSafeDragOutPath", () => {
it("keeps valid filenames inside the temp directory", () => {
const tempDir = path.join("tmp", "hypershell-drag");
const result = resolveSafeDragOutPath(tempDir, "server.log");
expect(result).toBe(path.resolve(tempDir, "server.log"));
});

it("rejects traversal and path separator payloads", () => {
expect(() => resolveSafeDragOutPath("/tmp/hypershell-drag", "../escape.txt")).toThrow(/invalid drag-out filename/i);
expect(() => resolveSafeDragOutPath("/tmp/hypershell-drag", "nested/escape.txt")).toThrow(/invalid drag-out filename/i);
});
});
111 changes: 90 additions & 21 deletions apps/desktop/src/main/ipc/sftpIpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ import {
type KeyboardInteractiveRequest,
} from "@hypershell/shared";
import type { SessionManager, SftpConnectionOptions, KeyboardInteractiveCallback } from "@hypershell/session-core";
import { createSyncEngine, probeHostKey } from "@hypershell/session-core";
import { createSyncEngine, probeHostKey, buildScpCommand } from "@hypershell/session-core";
import { execFile } from "node:child_process";
import { timingSafeEqual } from "node:crypto";
import type { IpcMainInvokeEvent } from "electron";

Expand All @@ -57,6 +58,28 @@ import { createSftpSessionManager, type SftpSessionManager } from "../sftp/sftpS
import { createTransferManager, type TransferManager } from "../sftp/transferManager";
import { createTransferManifest } from "../sftp/transferManifest";

export function resolveSafeDragOutPath(tempDir: string, fileName: string): string {
const trimmed = fileName.trim();
if (!trimmed) {
throw new Error("Invalid drag-out filename");
}

const baseName = path.basename(trimmed);
const hasPathSeparators = trimmed.includes(path.posix.sep) || trimmed.includes(path.win32.sep);
const hasControlChars = /[\0-\x1f]/.test(trimmed);
if (hasPathSeparators || hasControlChars || baseName !== trimmed || baseName === "." || baseName === "..") {
throw new Error("Invalid drag-out filename");
}

const resolvedTempDir = path.resolve(tempDir);
const resolvedTarget = path.resolve(resolvedTempDir, baseName);
if (resolvedTarget !== path.join(resolvedTempDir, baseName)) {
throw new Error("Invalid drag-out filename");
}

return resolvedTarget;
}

/**
* Error subclass thrown when host key verification fails.
* Contains the fingerprint details so the renderer can show the appropriate dialog.
Expand Down Expand Up @@ -188,6 +211,9 @@ export function registerSftpIpc(
const bookmarksRepo = createSftpBookmarksRepository(db);
const fingerprintRepo = createHostFingerprintRepositoryFromDatabase(db);

// Drag-out cache: pre-downloaded files keyed by "sessionId:remotePath"
const dragCache = new Map<string, string>();

// Keyboard-interactive auth relay: pending requests keyed by requestId
const pendingKbdInteractive = new Map<string, {
resolve: (responses: string[]) => void;
Expand Down Expand Up @@ -237,6 +263,11 @@ export function registerSftpIpc(
const hostname = connectOptions.hostname;
const port = connectOptions.port ?? 22;

const trustedFingerprints = fingerprintRepo
.findByHost(hostname, port)
.filter((record) => record.isTrusted)
.map((record) => record.fingerprint);

try {
const { algorithm, fingerprint } = await probeHostKey(hostname, port);
const existing = fingerprintRepo.findByHostAndAlgorithm(hostname, port, algorithm);
Expand Down Expand Up @@ -294,6 +325,7 @@ export function registerSftpIpc(

const sftpSessionId = await sftpSessionManager.connect(hostId, connectOptions, {
onKeyboardInteractive: createKeyboardInteractiveCallback(),
trustedHostFingerprints: trustedFingerprints,
});
options.onConnected?.({
sftpSessionId,
Expand Down Expand Up @@ -479,28 +511,65 @@ export function registerSftpIpc(

const handleDragOut = async (event: IpcMainInvokeEvent, rawRequest: unknown) => {
const request = sftpDragOutRequestSchema.parse(rawRequest);
const transport = sftpSessionManager.getTransport(request.sftpSessionId);
const cacheKey = `${request.sftpSessionId}:${request.remotePath}`;

// Check cache first — file may have been pre-downloaded on selection
let tempPath = dragCache.get(cacheKey);

if (!tempPath || !fs.existsSync(tempPath)) {
const session = sftpSessionManager.getSession(request.sftpSessionId);
if (!session) throw new Error(`SFTP session ${request.sftpSessionId} not found`);

const tempDir = path.join(app.getPath("temp"), "hypershell-drag");
fs.mkdirSync(tempDir, { recursive: true });
tempPath = resolveSafeDragOutPath(tempDir, request.fileName);

const connOpts = session.connectionOptions;
const keyPath = connOpts.privateKeyPath ?? connOpts.fallbackKeyPaths?.[0];
const scpCmd = buildScpCommand({
hostname: connOpts.hostname,
port: connOpts.port,
username: connOpts.username,
privateKeyPath: keyPath,
proxyJump: connOpts.proxyJump,
direction: "download",
remotePath: request.remotePath,
localPath: tempPath,
});
if (request.isDirectory) {
scpCmd.args.unshift("-r");
}

// Download to a temp directory
const tempDir = path.join(app.getPath("temp"), "hypershell-drag");
fs.mkdirSync(tempDir, { recursive: true });
const tempPath = path.join(tempDir, request.fileName);

// Stream remote file to local temp
await new Promise<void>((resolve, reject) => {
const readStream = transport.createReadStream(request.remotePath);
const writeStream = fs.createWriteStream(tempPath);
readStream.pipe(writeStream);
writeStream.on("finish", resolve);
writeStream.on("error", reject);
readStream.on("error", reject);
});
try {
await new Promise<void>((resolve, reject) => {
execFile(scpCmd.command, scpCmd.args, { windowsHide: true }, (error) => {
if (error) reject(error);
else resolve();
});
});
dragCache.set(cacheKey, tempPath);
} catch {
// SCP failed (directory, permission denied, etc.) — skip caching
return { tempPath: "" };
}
}

// Initiate native OS drag from the temp file
event.sender.startDrag({
file: tempPath,
icon: await app.getFileIcon(tempPath, { size: "small" }),
});
// prepareOnly: just cache the file, don't initiate OS drag
if (request.prepareOnly) {
return { tempPath };
}

// Initiate native OS drag from the cached temp file
let icon: Electron.NativeImage;
try {
icon = await app.getFileIcon(tempPath, { size: "small" });
} catch {
const { nativeImage } = await import("electron");
icon = nativeImage.createFromBuffer(
Buffer.from("iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAAADklEQVQ4jWNgGAWDEwAAAhAAATHKfqoAAAAASUVORK5CYII=", "base64")
);
}
event.sender.startDrag({ file: tempPath, icon });

return { tempPath };
};
Expand Down
4 changes: 3 additions & 1 deletion apps/desktop/src/main/main.lifecycle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ function createWindow(id: string) {
return this.destroyed;
},
webContents: {
send: vi.fn()
send: vi.fn(),
on: vi.fn(),
setWindowOpenHandler: vi.fn()
}
};
}
Expand Down
34 changes: 18 additions & 16 deletions apps/desktop/src/main/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { performAutoBackup } from "./ipc/backupIpc";
import { createAppMenu } from "./menu/createAppMenu";
import { createTray } from "./tray/createTray";
import { createMainWindow } from "./windows/createMainWindow";
import { assertAllowedRendererUrl } from "./windows/windowSecurity";
import { createMainProcessLifecycle } from "./mainLifecycle";
import { clearAll as clearCredentialCache } from "./security/credentialCache";
import { getOrCreateDatabase, getOrCreateHostsRepo } from "./ipc/hostsIpc";
Expand Down Expand Up @@ -86,22 +87,23 @@ function persistSessionRecoverySnapshot(): void {
}

function getRendererUrl(): string {
if (process.env.SSHTERM_RENDERER_URL) {
return process.env.SSHTERM_RENDERER_URL;
}

const bundledRendererEntry = path.join(
import.meta.dirname,
"..",
"renderer",
"index.html"
);

if (existsSync(bundledRendererEntry)) {
return pathToFileURL(bundledRendererEntry).toString();
}

return "http://localhost:5173";
const rawUrl = process.env.SSHTERM_RENDERER_URL
?? (() => {
const bundledRendererEntry = path.join(
import.meta.dirname,
"..",
"renderer",
"index.html"
);

if (existsSync(bundledRendererEntry)) {
return pathToFileURL(bundledRendererEntry).toString();
}

return "http://localhost:5173";
})();

return assertAllowedRendererUrl(rawUrl).toString();
}

const mainProcessLifecycle = createMainProcessLifecycle({
Expand Down
5 changes: 4 additions & 1 deletion apps/desktop/src/main/mainLifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type {
RegisterIpcOptions
} from "./ipc/registerIpc";
import { editorWindowManager } from "./windows/editorWindowManager";
import { attachWindowSecurityGuards } from "./windows/windowSecurity";
import type { TrayWindowLike } from "./tray/createTray";

interface ElectronAppLike {
Expand Down Expand Up @@ -81,7 +82,9 @@ export function createMainProcessLifecycle(
loadURL(url: string): Promise<void> | void;
} {
const window = deps.createMainWindow();
void window.loadURL(deps.getRendererUrl());
const rendererUrl = deps.getRendererUrl();
attachWindowSecurityGuards(window as unknown as Electron.BrowserWindow, rendererUrl);
void window.loadURL(rendererUrl);
mainWindow = window;
return window;
}
Expand Down
2 changes: 2 additions & 0 deletions apps/desktop/src/main/windows/createEditorWindow.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { BrowserWindow } from "electron";
import path from "node:path";
import { resolveAppIconPath } from "./resolveAppIconPath";
import { attachWindowSecurityGuards } from "./windowSecurity";

export interface CreateEditorWindowOptions {
sftpSessionId: string;
Expand Down Expand Up @@ -34,6 +35,7 @@ export function createEditorWindow(options: CreateEditorWindowOptions): BrowserW
const url = new URL(rendererUrl);
url.searchParams.set("window", "editor");
url.searchParams.set("sftpSessionId", sftpSessionId);
attachWindowSecurityGuards(win, rendererUrl);
void win.loadURL(url.toString());

return win;
Expand Down
58 changes: 58 additions & 0 deletions apps/desktop/src/main/windows/windowSecurity.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { describe, expect, it, vi } from "vitest";

import {
assertAllowedRendererUrl,
attachWindowSecurityGuards,
isAllowedNavigationTarget,
} from "./windowSecurity";

describe("assertAllowedRendererUrl", () => {
it("accepts local dev and file renderer URLs", () => {
expect(assertAllowedRendererUrl("http://localhost:5173").toString()).toBe("http://localhost:5173/");
expect(assertAllowedRendererUrl("http://127.0.0.1:5173").toString()).toBe("http://127.0.0.1:5173/");
expect(assertAllowedRendererUrl("file:///tmp/renderer/index.html").toString()).toBe("file:///tmp/renderer/index.html");
});

it("rejects remote renderer URLs", () => {
expect(() => assertAllowedRendererUrl("https://evil.example/app")).toThrow(/renderer url/i);
});
});

describe("isAllowedNavigationTarget", () => {
it("allows same-origin dev navigation and same-file production navigation", () => {
expect(isAllowedNavigationTarget("http://localhost:5173", "http://localhost:5173/?window=editor")).toBe(true);
expect(isAllowedNavigationTarget("file:///tmp/renderer/index.html", "file:///tmp/renderer/index.html?window=editor")).toBe(true);
});

it("rejects cross-origin and cross-file navigation", () => {
expect(isAllowedNavigationTarget("http://localhost:5173", "https://evil.example/")).toBe(false);
expect(isAllowedNavigationTarget("file:///tmp/renderer/index.html", "file:///tmp/renderer/other.html")).toBe(false);
});
});

describe("attachWindowSecurityGuards", () => {
it("blocks unexpected navigation and denies new windows", () => {
const willNavigateHandlers: Array<(event: { preventDefault: () => void }, url: string) => void> = [];
const setWindowOpenHandler = vi.fn();

const fakeWindow = {
webContents: {
on: vi.fn((event: string, handler: (event: { preventDefault: () => void }, url: string) => void) => {
if (event === "will-navigate") {
willNavigateHandlers.push(handler);
}
}),
setWindowOpenHandler,
},
} as const;

attachWindowSecurityGuards(fakeWindow as never, "http://localhost:5173");

const preventDefault = vi.fn();
willNavigateHandlers[0]?.({ preventDefault }, "https://evil.example/");
expect(preventDefault).toHaveBeenCalledTimes(1);

const handler = setWindowOpenHandler.mock.calls[0]?.[0] as ((details: { url: string }) => { action: string });
expect(handler({ url: "http://localhost:5173/help" })).toEqual({ action: "deny" });
});
});
42 changes: 42 additions & 0 deletions apps/desktop/src/main/windows/windowSecurity.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
export function assertAllowedRendererUrl(rawUrl: string): URL {
const parsed = new URL(rawUrl);

if (parsed.protocol === "file:") {
return parsed;
}

const isLocalHttp = (parsed.protocol === "http:" || parsed.protocol === "https:")
&& (parsed.hostname === "localhost" || parsed.hostname === "127.0.0.1");

if (isLocalHttp) {
return parsed;
}

throw new Error(`Renderer URL is not allowed: ${rawUrl}`);
}

export function isAllowedNavigationTarget(rendererUrl: string, targetUrl: string): boolean {
const renderer = assertAllowedRendererUrl(rendererUrl);
const target = new URL(targetUrl);

if (renderer.protocol === "file:") {
return target.protocol === "file:" && target.pathname === renderer.pathname;
}

return target.origin === renderer.origin;
}

export function attachWindowSecurityGuards(
win: Pick<Electron.BrowserWindow, "webContents">,
rendererUrl: string
): void {
const allowedRendererUrl = assertAllowedRendererUrl(rendererUrl).toString();

win.webContents.on("will-navigate", (event, url) => {
if (!isAllowedNavigationTarget(allowedRendererUrl, url)) {
event.preventDefault();
}
});

win.webContents.setWindowOpenHandler(() => ({ action: "deny" }));
}
Loading
Loading