diff --git a/package.json b/package.json index a5f1c30..16c8184 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "preview": "electron-vite preview", "typecheck": "tsc --noEmit -p tsconfig.node.json && tsc --noEmit -p tsconfig.web.json", "pretest": "npm run typecheck", - "test": "node tests/build-bundles.cjs && node tests/ssh-loopback.mjs && node tests/commands-store.mjs && node tests/connections-store.mjs && node tests/settings-store.mjs && node tests/lock-store.mjs && node tests/lock-controller.mjs && node tests/lock-shortcuts.mjs && node tests/.hl-split-smoke.cjs && node tests/.hl-rules.cjs && node tests/zmodem-e2e.mjs && node tests/ssh-session-e2e.mjs && node tests/sysinfo-e2e.mjs", + "test": "node tests/build-bundles.cjs && node tests/ssh-loopback.mjs && node tests/commands-store.mjs && node tests/connections-store.mjs && node tests/settings-store.mjs && node tests/local-path-grants.mjs && node tests/lock-store.mjs && node tests/lock-controller.mjs && node tests/lock-shortcuts.mjs && node tests/.hl-split-smoke.cjs && node tests/.hl-rules.cjs && node tests/zmodem-e2e.mjs && node tests/ssh-session-e2e.mjs && node tests/sysinfo-e2e.mjs", "predist": "npm test && npm install --package-lock-only", "dist": "electron-vite build && electron-builder --win msi nsis", "dist:dir": "electron-vite build && electron-builder --win --dir" diff --git a/src/main/index.ts b/src/main/index.ts index d9f93ab..35021a9 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -157,6 +157,14 @@ function createWindow(): void { ...(existsSync(devIcon) ? { icon: nativeImage.createFromPath(devIcon) } : {}), webPreferences: { preload: join(__dirname, '../preload/index.js'), + // Written out explicitly even though every value matches Electron's + // current default: these are the three that keep the renderer unable to + // reach the main process, and a default that silently changed (or a + // future Electron release flipping one) must not be the only thing + // holding that line. See also the sender guard in ipc.ts. + contextIsolation: true, + nodeIntegration: false, + webSecurity: true, // Keep the default renderer sandbox (preload only touches the electron // IPC bridge, so it does not need Node access). sandbox: true diff --git a/src/main/localPathGrants.ts b/src/main/localPathGrants.ts new file mode 100644 index 0000000..0a3ea6b --- /dev/null +++ b/src/main/localPathGrants.ts @@ -0,0 +1,165 @@ +/** + * Local-path admission for the files the main process is asked to read or write + * on behalf of the renderer (SFTP transfers, ZMODEM send/receive). + * + * Threat model: the renderer is the untrusted half of the app. Every local path + * it sends over IPC used to be taken at face value, so a compromised renderer + * (a navigation that slipped past the frame guard, a future caller that forgets + * the dialog) could make the main process read or write anywhere the user can + * reach. Local paths are therefore *granted* first, and the only grant source is + * a native file / directory dialog (sftp.ts's pickFiles / pickDirectory) — an + * explicit choice the user made with their own hands. + * + * Granting and checking both go through realpath: the registry stores resolved + * paths, and a candidate that does not resolve (missing file, dead drive, + * unreadable parent) is never granted and never accepted. Comparing only the + * literal spelling would let a symlinked parent steer a path that *looks* like + * it sits inside a granted directory somewhere else entirely — the resolved path + * is the only one that answers "where would this actually read/write?". + * + * Grant lifetime is the process: a directory the user picked as a save target + * stays valid for later transfers into it. Each dialog returns at most a handful + * of paths, so both sets stay tiny. + * + * `grantedPaths` is the real policy and the DEFAULT for every consumer, so a + * caller that forgets to inject one is still enforced. `LocalPathPolicy` exists + * only so the offline test harnesses (which have no dialog to grant from) can + * supply a double; it is never reachable from renderer input. + */ +import { realpathSync, statSync } from 'fs' +import { basename, join, sep } from 'path' + +/** + * What the transfer engines need to know about local paths. Implemented by + * `grantedPaths` below; tests inject a permissive double. + * + * The methods return the *resolved* path to use rather than a boolean, so the + * caller opens exactly what was checked — re-joining or re-reading the original + * spelling would reopen the symlink window the check just closed. + */ +export interface LocalPathPolicy { + /** The local file an upload may read, or null when it must be refused. */ + readSource(path: unknown): string | null + /** The local directory a download may be written into, or null. */ + readDirectory(dir: unknown): string | null + /** + * The local path a write of `fileName` into `dir` may go to, or null when it + * must be refused. `fileName` comes off the wire (a remote file's basename, + * or the name a ZMODEM peer offered). + */ + writeTarget(dir: unknown, fileName: unknown): string | null +} + +const grantedFiles = new Set() +const grantedDirs = new Set() + +/** + * Windows compares paths case-insensitively; drive-letter and name casing must + * not decide whether a granted path matches. Applied to comparisons only — a + * path handed back to a caller keeps the casing the filesystem reported. + */ +const fold = (p: string): string => (process.platform === 'win32' ? p.toLowerCase() : p) + +/** Real path of an existing regular file, or null. */ +function realFile(p: unknown): string | null { + if (typeof p !== 'string' || p === '') return null + try { + const real = realpathSync(p) + return statSync(real).isFile() ? real : null + } catch { + return null + } +} + +/** Real path of an existing directory, or null. */ +function realDir(p: unknown): string | null { + if (typeof p !== 'string' || p === '') return null + try { + const real = realpathSync(p) + return statSync(real).isDirectory() ? real : null + } catch { + return null + } +} + +/** Whether a folded, already-resolved directory sits at or under a granted dir. */ +function isGrantedKey(key: string): boolean { + for (const dir of grantedDirs) { + if (key === dir || key.startsWith(dir + sep)) return true + } + return false +} + +/** A single path segment: no separators, no `.`/`..`, not empty. */ +function isPlainLeaf(name: unknown): name is string { + return ( + typeof name === 'string' && + name !== '' && + name !== '.' && + name !== '..' && + basename(name) === name + ) +} + +/** + * Grant the files a native "open file" dialog just returned. A non-file in the + * list (a path that vanished between the dialog and here) grants nothing — + * everything reaches the engine through realpath, so an unresolvable entry can + * never match later either. + */ +export function grantPickedFiles(paths: unknown): void { + if (!Array.isArray(paths)) return + for (const p of paths) { + const real = realFile(p) + if (real !== null) grantedFiles.add(fold(real)) + } +} + +/** Grant the directory a native "open directory" dialog just returned. */ +export function grantPickedDirectory(dir: unknown): void { + const real = realDir(dir) + if (real !== null) grantedDirs.add(fold(real)) +} + +/** + * The real policy. Every method re-resolves the path at check time: a file that + * has since been deleted, replaced by a directory, or had a symlink swapped in + * over it fails, even though it was granted earlier. + */ +export const grantedPaths: LocalPathPolicy = { + readSource(path: unknown): string | null { + const real = realFile(path) + return real !== null && grantedFiles.has(fold(real)) ? real : null + }, + + readDirectory(dir: unknown): string | null { + const real = realDir(dir) + return real !== null && isGrantedKey(fold(real)) ? real : null + }, + + writeTarget(dir: unknown, fileName: unknown): string | null { + const dirReal = realDir(dir) + if (dirReal === null) return null + const key = fold(dirReal) + if (!isGrantedKey(key)) return null + // Checked as a plain leaf first: a name still carrying a separator, or the + // bare `..`, must not climb out of the directory the user chose. + if (!isPlainLeaf(fileName)) return null + // The join is made against the *resolved* directory, so the containment + // check and the actual write agree on the target. + const target = join(dirReal, fileName) + let resolved: string + try { + resolved = realpathSync(target) + } catch { + // Not on disk yet: a fresh file created directly inside the granted dir. + return target + } + // Something is already there. Resolve it too: a symlink planted at that + // name would otherwise land the bytes somewhere else entirely while the + // write still looks like it targets the download folder. + const targetKey = fold(resolved) + if (targetKey === key || !targetKey.startsWith(key + sep)) return null + return target + } +} diff --git a/src/main/sftp.ts b/src/main/sftp.ts index 116f3a7..fec8a31 100644 --- a/src/main/sftp.ts +++ b/src/main/sftp.ts @@ -2,10 +2,28 @@ import { dialog } from 'electron' import type { Client, SFTPWrapper } from 'ssh2' import { randomUUID } from 'crypto' import { createWriteStream, promises as fsp } from 'fs' -import { basename, join } from 'path' +import { basename } from 'path' import { Ipc } from '../shared/ipc' import { t } from '../shared/i18n' import type { SftpEntry, TransferProgressEvent } from '../shared/sftp' +import { + grantPickedDirectory, + grantPickedFiles, + grantedPaths, + type LocalPathPolicy +} from './localPathGrants' + +/** + * Local-path admission for this module's transfers. Defaults to the real grant + * registry — a caller that forgets to inject one is still enforced — and is + * only swapped by the offline test harnesses, which have no dialog to grant + * from. Never fed from renderer input. + */ +let pathPolicy: LocalPathPolicy = grantedPaths + +export function setLocalPathPolicy(policy: LocalPathPolicy): void { + pathPolicy = policy +} /** ssh2 Client lookup injected by pty.ts (kind === 'ssh' sessions only). */ let clientProvider: ((id: string) => Client | undefined) | undefined @@ -347,12 +365,74 @@ type Broadcast = (channel: string, payload: unknown) => void const CHUNK = 256 * 1024 const UPLOAD_WINDOW = 64 +/** + * Path checks for the local side of a transfer. + * + * Threat model: `localPaths` and `localDir` arrive from the renderer, which is + * untrusted — without a check the main process would read or write any path the + * user can reach (a compromised renderer, a hand-crafted IPC message, or a + * future caller that forgets the dialog). Only paths the user granted through a + * native dialog are accepted; see localPathGrants.ts for what a grant is and + * why it is realpath-based. + * + * Both helpers throw, so the invoke rejects right away and the renderer + * surfaces the reason instead of starting a transfer that fails later. The + * checks run *before* the sftp channel is opened, so a refused request never + * costs a subsystem channel. Each returns the *resolved* path to open: the + * caller must use that, not the renderer's spelling, or it would re-traverse + * the very symlink the check followed. + */ +function requireUploadSources(localPaths: unknown): { source: string; name: string }[] { + if (!Array.isArray(localPaths) || localPaths.length === 0) { + throw new Error(t('main.sftp.localNotAllowed', { path: '' })) + } + return localPaths.map(local => { + // readSource also insists on a real regular file: a directory or a device + // node must never reach the upload's file handle. + const source = pathPolicy.readSource(local) + if (source === null) { + throw new Error(t('main.sftp.localNotAllowed', { path: String(local) })) + } + // The remote name stays the one derived from the path the USER saw in the + // dialog, not from the resolved target: FilePanel computes its overwrite + // confirmation from the same spelling, and a name that differed from the + // one the user approved would let an upload overwrite a file it never + // warned about. Only the path that gets READ is the resolved one. + return { source, name: basename(String(local)) } + }) +} + +/** + * Where each remote file lands locally. The directory must be one the user + * granted, and the name is the remote file's basename — checked as a plain leaf + * with any symlink already sitting at that name resolved (see + * localPathGrants.ts). + */ +function requireDownloadTargets( + remotePaths: unknown, + localDir: unknown +): { remote: string; name: string; target: string }[] { + if (!Array.isArray(remotePaths) || remotePaths.length === 0) { + throw new Error(t('main.sftp.localNotAllowed', { path: String(localDir) })) + } + return remotePaths.map(remote => { + const path = String(remote) + const name = basename(path) + const target = pathPolicy.writeTarget(localDir, name) + if (target === null) { + throw new Error(t('main.sftp.localNotAllowed', { path: String(localDir) })) + } + return { remote: path, name, target } + }) +} + export function uploadRemote( sessionId: string, localPaths: string[], remoteDir: string, broadcast: Broadcast ): Promise { + const sources = requireUploadSources(localPaths) return withSftp(sessionId, async sftp => { const id = randomUUID() const transfer: ActiveTransfer = { kind: 'upload', cancelled: false } @@ -361,9 +441,8 @@ export function uploadRemote( void (async () => { try { - for (const local of localPaths) { + for (const { source: local, name } of sources) { if (transfer.cancelled) break - const name = basename(local) const size = (await fsp.stat(local)).size const remote = `${remoteDir.replace(/\/+$/, '')}/${name}` const handle = await p(cb => sftp.open(remote, 'w', cb)) @@ -447,6 +526,7 @@ export function downloadRemote( localDir: string, broadcast: Broadcast ): Promise { + const targets = requireDownloadTargets(remotePaths, localDir) return withSftp(sessionId, async sftp => { const id = randomUUID() const transfer: ActiveTransfer = { kind: 'download', cancelled: false } @@ -455,11 +535,9 @@ export function downloadRemote( void (async () => { try { - for (const remote of remotePaths) { + for (const { remote, name, target: local } of targets) { if (transfer.cancelled) break - const name = basename(remote) const { size } = await p<{ size: number }>(cb => sftp.stat(remote, cb)) - const local = join(localDir, name) const handle = await p(cb => sftp.open(remote, 'r', cb)) try { const localHandle = await fsp.open(local, 'w') @@ -516,13 +594,24 @@ export function downloadRemote( }) } -/** Native file dialogs parented to the focused window. */ +/** + * Native file dialogs parented to the focused window. + * + * These two are the ONLY grant source (see localPathGrants.ts): whatever the + * user picks here becomes usable by the transfer paths, and nothing else does. + * A cancelled dialog grants nothing — the empty result must not be read as + * "no restriction". Both return `filePaths` verbatim so the renderer's own + * display logic is unchanged. + */ export async function pickFiles(): Promise { const r = await dialog.showOpenDialog({ properties: ['openFile', 'multiSelections'] }) + grantPickedFiles(r.filePaths) return r.filePaths } export async function pickDirectory(): Promise { const r = await dialog.showOpenDialog({ properties: ['openDirectory'] }) - return r.filePaths[0] ?? '' + const dir = r.filePaths[0] ?? '' + if (dir !== '') grantPickedDirectory(dir) + return dir } diff --git a/src/main/ssh.ts b/src/main/ssh.ts index 86dbf52..8726469 100644 --- a/src/main/ssh.ts +++ b/src/main/ssh.ts @@ -16,11 +16,20 @@ import type { SshConnection, SshSecretOverride } from '../shared/connections' import { t } from '../shared/i18n' import { randomUUID } from 'crypto' -import { readFileSync } from 'fs' +import { readFileSync, realpathSync, statSync } from 'fs' import { fingerprintOf, type HostKeyCheckResult } from './knownHosts' import type { ConnectionsStore } from './connectionsStore' import type { Client, ClientChannel, ConnectConfig } from 'ssh2' +/** + * Private keys are a few KB; a file past this is a mistyped path (a disk image, + * a log), and reading it would put the whole thing on the UI thread's heap. + */ +const MAX_KEY_BYTES = 1024 * 1024 + +/** A refusal we produced ourselves: the message is already user-complete. */ +class RefusedKeyError extends Error {} + export interface SshSessionHandle { id: string /** established ssh connection; powers the session routing in pty.ts */ @@ -350,11 +359,37 @@ export function resolveHostKey(promptId: string, action: 'accept' | 'reject'): v pending.resolve(action === 'accept') } -/** Read a private key file; surfaces a descriptive error on failure. */ +/** + * Read a private key file, refusing anything that is not a regular file. + * + * Threat model: `keyPath` reaches here from the renderer (typed into the edit + * dialog, persisted in connections.json, then echoed back on connect), and it + * lands in a synchronous read on the UI thread. Two failures there would be + * fatal to the app, not just to this connection: + * + * - a device or FIFO (`/dev/zero`, `\\.\pipe\...`) makes the read block + * forever — the main process stops answering, and nothing times it out. A + * directory at least fails, but only with a confusing EISDIR. + * - a huge file (a disk image, a log) is slurped into memory in one call, so + * a mistyped path can exhaust the heap. + * + * A symlink is a third problem: the OS follows it, so the file actually read is + * not the file that was named — and a link the user did not create could point + * at anything. Resolving with realpath first makes the check and the read agree + * on one path, and the read then uses that resolved path. + */ function readKeyFile(keyPath: string): string { try { - return readFileSync(keyPath, 'utf8') + const real = realpathSync(keyPath) + const st = statSync(real) + if (!st.isFile()) throw new RefusedKeyError(t('main.ssh.keyNotAFile')) + if (st.size > MAX_KEY_BYTES) throw new RefusedKeyError(t('main.ssh.keyTooLarge')) + return readFileSync(real, 'utf8') } catch (err) { - throw new Error(t('main.ssh.readKeyFailed', { path: keyPath, detail: (err as Error).message })) + // A refusal is already a complete message; an I/O error has only an errno + // to contribute, so it gets wrapped with the path the user typed. + if (err instanceof RefusedKeyError) throw err + const detail = err instanceof Error ? err.message : String(err) + throw new Error(t('main.ssh.readKeyFailed', { path: keyPath, detail })) } } \ No newline at end of file diff --git a/src/main/zmodem.ts b/src/main/zmodem.ts index 2846f99..36b4b7d 100644 --- a/src/main/zmodem.ts +++ b/src/main/zmodem.ts @@ -15,7 +15,7 @@ * reach the terminal), and pty.ts's writePty drops user keystrokes via * isZmodemActive. */ -import { basename, join } from 'path' +import { basename } from 'path' import { createReadStream, createWriteStream } from 'fs' import { stat } from 'fs/promises' import type { Writable } from 'stream' @@ -24,6 +24,19 @@ import { Ipc } from '../shared/ipc' import type { ZmodemDoneEvent, ZmodemResponse } from '../shared/ipc' import type { TransferProgressEvent } from '../shared/sftp' import { t } from '../shared/i18n' +import { grantedPaths, type LocalPathPolicy } from './localPathGrants' + +/** + * Local-path admission, same shape and same rationale as sftp.ts's: defaults to + * the real grant registry so a caller that forgets to inject one is still + * enforced, and the offline e2e harness swaps in a double because it has no + * dialog to grant from. Never fed from renderer input. + */ +let pathPolicy: LocalPathPolicy = grantedPaths + +export function setLocalPathPolicy(policy: LocalPathPolicy): void { + pathPolicy = policy +} /** How long to wait for the renderer to answer a ZMODEM_OFFER (ms). */ const OFFER_TIMEOUT_MS = 120_000 @@ -293,6 +306,11 @@ async function sendOneFile( }) } +/** + * Upload the approved local files. `paths` are the paths the policy accepted + * *and* resolved, so `createReadStream` opens exactly what was checked; the + * name offered to the peer is the basename of that resolved path. + */ async function runSend(engine: Engine, paths: string[]): Promise { try { const session = engine.session as Zmodem.SendSession @@ -332,8 +350,18 @@ async function runSend(engine: Engine, paths: string[]): Promise { function handleOffer(engine: Engine, offer: Zmodem.Offer): void { const details = offer.get_details() + // The offered name is the PEER's string, so it is display data only: the path + // actually written is decided by the policy below, which refuses anything that + // is not a plain leaf inside the directory the user granted. `basename` keeps + // the UI honest for a peer that sends a path rather than a name. const name = basename(String(details.name ?? 'file')) - const dest = join(engine.dir ?? '.', name) + const dest = pathPolicy.writeTarget(engine.dir, name) + if (dest === null) { + // Refusing one file must not silently look like success: end the transfer + // with the reason, which also stops the peer (finalize aborts the session). + finalize(engine, false, t('main.zmodem.savePathNotAllowed', { name })) + return + } if (details.size) engine.totalBytes += details.size const stream = createWriteStream(dest) @@ -596,7 +624,15 @@ export function respondZmodem(resp: ZmodemResponse): void { abortSession(engine, t('main.zmodem.noSaveDir')) return } - engine.dir = resp.dir + // The save directory came from the renderer: only one the user granted + // through the dialog is usable (localPathGrants.ts). Resolved once here, so + // every file of the transfer is written into the directory that was checked. + const dir = pathPolicy.readDirectory(resp.dir) + if (dir === null) { + abortSession(engine, t('main.zmodem.saveDirNotAllowed')) + return + } + engine.dir = dir const session = engine.session as Zmodem.ReceiveSession session.on('offer', (offer) => handleOffer(engine, offer)) void session @@ -605,12 +641,27 @@ export function respondZmodem(resp: ZmodemResponse): void { finalize(engine, false, err instanceof Error ? err.message : t('main.zmodem.receiveFailed')) ) } else { - const paths = resp.paths ?? [] + // Same for the files to send: the renderer supplies the paths, so each one + // must be a file the user picked. Checked before the transfer starts — + // rejecting mid-stream would leave the peer waiting on an abort. + const paths = Array.isArray(resp.paths) ? resp.paths : [] if (paths.length === 0) { abortSession(engine, t('main.zmodem.noFiles')) return } - void runSend(engine, paths) + // All or nothing. Dropping just the refused entries would run a transfer + // that reports success while quietly shipping fewer files than the user + // selected. + const allowed: string[] = [] + for (const p of paths) { + const source = pathPolicy.readSource(p) + if (source === null) { + abortSession(engine, t('main.zmodem.filesNotAllowed')) + return + } + allowed.push(source) + } + void runSend(engine, allowed) } } diff --git a/src/shared/i18n/dicts/en/main.ts b/src/shared/i18n/dicts/en/main.ts index 33c90d4..7af12d6 100644 --- a/src/shared/i18n/dicts/en/main.ts +++ b/src/shared/i18n/dicts/en/main.ts @@ -26,6 +26,8 @@ const main: Record = { 'main.ssh.shellOpenFailed': 'Could not open the SSH shell ({host}:{port}): {detail}', 'main.ssh.initFailed': 'SSH connection initialization failed ({host}:{port}): {detail}', 'main.ssh.readKeyFailed': 'Could not read the private key file {path}: {detail}', + 'main.ssh.keyNotAFile': 'The private key path is not a regular file (directories, devices and other special files are refused)', + 'main.ssh.keyTooLarge': 'The private key file is too large - the path is probably wrong', 'main.sftp.sessionGone': 'The SSH session does not exist or has disconnected', 'main.sftp.deleteFailed': 'Delete failed: {detail}', @@ -35,6 +37,7 @@ const main: Record = { 'main.sftp.openTimeout': 'Opening the SFTP channel timed out', 'main.sftp.commandExitCode': 'Command exited with code {code}', 'main.sftp.cancelled': 'Cancelled', + 'main.sftp.localNotAllowed': 'Local path not authorised: {path}. Pick it again with the "choose file / choose directory" dialog.', 'main.zmodem.transferFailed': 'Transfer failed', 'main.zmodem.transferTimeout': 'Transfer timed out', @@ -49,6 +52,9 @@ const main: Record = { 'main.zmodem.sessionCreateFailed': 'Could not establish the ZMODEM session', 'main.zmodem.noSaveDir': 'No save directory specified', 'main.zmodem.noFiles': 'No files selected', + 'main.zmodem.saveDirNotAllowed': 'The save directory is not authorised. Pick it again with the "choose directory" dialog.', + 'main.zmodem.savePathNotAllowed': 'The remote file name was refused, transfer aborted: {name}', + 'main.zmodem.filesNotAllowed': 'The selected local files are not authorised, transfer aborted', 'main.ipc.connectionMissing': 'Connection bookmark not found ({id})', diff --git a/src/shared/i18n/dicts/ja/main.ts b/src/shared/i18n/dicts/ja/main.ts index ad9f5d4..ddc5492 100644 --- a/src/shared/i18n/dicts/ja/main.ts +++ b/src/shared/i18n/dicts/ja/main.ts @@ -26,6 +26,8 @@ const main: Record = { 'main.ssh.shellOpenFailed': 'SSH シェルを開けません ({host}:{port}): {detail}', 'main.ssh.initFailed': 'SSH 接続の初期化に失敗しました ({host}:{port}): {detail}', 'main.ssh.readKeyFailed': '秘密鍵ファイルを読み取れません {path}: {detail}', + 'main.ssh.keyNotAFile': '秘密鍵のパスが通常のファイルではありません(ディレクトリ・デバイス・その他の特殊ファイルは受け付けません)', + 'main.ssh.keyTooLarge': '秘密鍵ファイルが大きすぎます。パスの指定が誤っている可能性があります', 'main.sftp.sessionGone': 'SSH セッションが存在しないか、切断されています', 'main.sftp.deleteFailed': '削除に失敗しました: {detail}', @@ -35,6 +37,7 @@ const main: Record = { 'main.sftp.openTimeout': 'SFTP チャネルのオープンがタイムアウトしました', 'main.sftp.commandExitCode': 'コマンドの終了コード {code}', 'main.sftp.cancelled': 'キャンセルしました', + 'main.sftp.localNotAllowed': 'ローカルパスが許可されていません: {path}。「ファイルを選択 / ディレクトリを選択」ダイアログで選び直してください。', 'main.zmodem.transferFailed': '転送に失敗しました', 'main.zmodem.transferTimeout': '転送がタイムアウトしました', @@ -49,6 +52,9 @@ const main: Record = { 'main.zmodem.sessionCreateFailed': 'ZMODEM セッションを確立できません', 'main.zmodem.noSaveDir': '保存先ディレクトリが指定されていません', 'main.zmodem.noFiles': 'ファイルが選択されていません', + 'main.zmodem.saveDirNotAllowed': '保存先ディレクトリが許可されていません。「ディレクトリを選択」ダイアログで選び直してください。', + 'main.zmodem.savePathNotAllowed': 'リモートのファイル名を受け付けられないため、転送を中止しました: {name}', + 'main.zmodem.filesNotAllowed': '選択されたローカルファイルが許可されていないため、転送を中止しました', 'main.ipc.connectionMissing': '接続ブックマークが見つかりません ({id})', diff --git a/src/shared/i18n/dicts/zh-CN/main.ts b/src/shared/i18n/dicts/zh-CN/main.ts index 83b6dfc..13b1660 100644 --- a/src/shared/i18n/dicts/zh-CN/main.ts +++ b/src/shared/i18n/dicts/zh-CN/main.ts @@ -26,6 +26,8 @@ const main: Record = { 'main.ssh.shellOpenFailed': '无法打开 SSH shell ({host}:{port}): {detail}', 'main.ssh.initFailed': 'SSH 连接初始化失败 ({host}:{port}): {detail}', 'main.ssh.readKeyFailed': '无法读取私钥文件 {path}: {detail}', + 'main.ssh.keyNotAFile': '私钥路径不是一个普通文件(目录、设备或其他特殊文件均不被接受)', + 'main.ssh.keyTooLarge': '私钥文件过大,疑似路径填写有误', 'main.sftp.sessionGone': 'SSH 会话不存在或已断开', 'main.sftp.deleteFailed': '删除失败: {detail}', @@ -35,6 +37,7 @@ const main: Record = { 'main.sftp.openTimeout': 'SFTP 通道打开超时', 'main.sftp.commandExitCode': '命令退出码 {code}', 'main.sftp.cancelled': '已取消', + 'main.sftp.localNotAllowed': '本地路径未被授权: {path}。请通过「选择文件 / 选择目录」对话框重新选择。', 'main.zmodem.transferFailed': '传输失败', 'main.zmodem.transferTimeout': '传输超时', @@ -49,6 +52,9 @@ const main: Record = { 'main.zmodem.sessionCreateFailed': '无法建立 ZMODEM 会话', 'main.zmodem.noSaveDir': '未指定保存目录', 'main.zmodem.noFiles': '未选择文件', + 'main.zmodem.saveDirNotAllowed': '保存目录未被授权,请通过「选择目录」对话框重新选择', + 'main.zmodem.savePathNotAllowed': '远端文件名不被接受,已中止传输: {name}', + 'main.zmodem.filesNotAllowed': '所选的本地文件未被授权,已中止传输', 'main.ipc.connectionMissing': '连接书签不存在 ({id})', diff --git a/src/shared/i18n/dicts/zh-TW/main.ts b/src/shared/i18n/dicts/zh-TW/main.ts index 63b0bcb..6ee4352 100644 --- a/src/shared/i18n/dicts/zh-TW/main.ts +++ b/src/shared/i18n/dicts/zh-TW/main.ts @@ -26,6 +26,8 @@ const main: Record = { 'main.ssh.shellOpenFailed': '無法開啟 SSH shell ({host}:{port}): {detail}', 'main.ssh.initFailed': 'SSH 連線初始化失敗 ({host}:{port}): {detail}', 'main.ssh.readKeyFailed': '無法讀取私密金鑰檔案 {path}: {detail}', + 'main.ssh.keyNotAFile': '私密金鑰路徑不是一般檔案(目錄、裝置或其他特殊檔案均不接受)', + 'main.ssh.keyTooLarge': '私密金鑰檔案過大,路徑可能填寫有誤', 'main.sftp.sessionGone': 'SSH 連線不存在或已中斷', 'main.sftp.deleteFailed': '刪除失敗: {detail}', @@ -35,6 +37,7 @@ const main: Record = { 'main.sftp.openTimeout': 'SFTP 通道開啟逾時', 'main.sftp.commandExitCode': '指令結束碼 {code}', 'main.sftp.cancelled': '已取消', + 'main.sftp.localNotAllowed': '本機路徑未獲授權: {path}。請改用「選擇檔案 / 選擇目錄」對話框重新選擇。', 'main.zmodem.transferFailed': '傳輸失敗', 'main.zmodem.transferTimeout': '傳輸逾時', @@ -49,6 +52,9 @@ const main: Record = { 'main.zmodem.sessionCreateFailed': '無法建立 ZMODEM 連線', 'main.zmodem.noSaveDir': '未指定儲存目錄', 'main.zmodem.noFiles': '未選擇檔案', + 'main.zmodem.saveDirNotAllowed': '儲存目錄未獲授權,請改用「選擇目錄」對話框重新選擇', + 'main.zmodem.savePathNotAllowed': '遠端檔案名稱不被接受,已中止傳輸: {name}', + 'main.zmodem.filesNotAllowed': '所選的本機檔案未獲授權,已中止傳輸', 'main.ipc.connectionMissing': '連線書籤不存在 ({id})', diff --git a/tests/build-bundles.cjs b/tests/build-bundles.cjs index cd40381..48fbe26 100644 --- a/tests/build-bundles.cjs +++ b/tests/build-bundles.cjs @@ -26,6 +26,8 @@ const BUNDLES = [ // Known-hosts store: TOFU / changed / unreadable fail-closed behavior. { entry: 'src/main/knownHosts.ts', out: 'tests/.known-hosts.cjs' }, { entry: 'src/main/settingsStore.ts', out: 'tests/.settings-store.cjs' }, + // Local-path admission (grants): pure fs/path, no electron surface at all. + { entry: 'src/main/localPathGrants.ts', out: 'tests/.local-path-grants.cjs' }, // Lock-password store: scrypt verifier, round trip, damaged-file handling. { entry: 'src/main/lockStore.ts', out: 'tests/.lock-store.cjs' }, // Lock controller: cooldown ladder, serialized attempts, persisted flags. diff --git a/tests/local-path-grants.mjs b/tests/local-path-grants.mjs new file mode 100644 index 0000000..88d7eb8 --- /dev/null +++ b/tests/local-path-grants.mjs @@ -0,0 +1,185 @@ +/** + * Local-path admission (local-path-grants.mjs). + * + * src/main/localPathGrants.ts is the check that stands between a renderer- + * supplied path and the main process's filesystem access, so its rules are + * pinned here against a real temp tree rather than a mocked `fs`: + * + * - nothing is usable before the user granted it through the dialog + * - a granted file is readable, a granted directory (and its subdirectories) + * is writable, and unrelated paths stay refused + * - a peer-supplied file name cannot climb out of the download directory + * (`../x`, an absolute path, a name with a separator, `.`/`..`, empty) + * - a symlink planted at the target name is resolved, so a link pointing + * outside the granted tree is refused while one pointing inside is not + * - a grant is re-checked on every use: a path that has since disappeared + * stops being valid + * - granting through the wrong dialog (a directory via pickFiles, a file via + * pickDirectory) grants nothing + * + * Build: node tests/build-bundles.cjs + * Run: node tests/local-path-grants.mjs (must exit 0) + */ +import { createRequire } from 'node:module' +import { execFileSync } from 'node:child_process' +import { existsSync, mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { dirname, join } from 'node:path' +import { fileURLToPath } from 'node:url' + +const __dirname = dirname(fileURLToPath(import.meta.url)) +const bundlePath = join(__dirname, '.local-path-grants.cjs') + +// Same self-build fallback as zmodem-e2e.mjs, so the file can be run on its own. +if (!existsSync(bundlePath)) { + console.log('[local-path-grants] building bundle ...') + const cmd = process.platform === 'win32' ? 'npx.cmd' : 'npx' + execFileSync( + cmd, + [ + 'esbuild', + join(__dirname, '../src/main/localPathGrants.ts'), + '--bundle', + '--platform=node', + '--format=cjs', + `--outfile=${bundlePath}` + ], + { stdio: 'inherit', shell: process.platform === 'win32' } + ) +} + +const require_ = createRequire(import.meta.url) +const { grantedPaths, grantPickedFiles, grantPickedDirectory } = require_(bundlePath) + +let failed = 0 +const ok = (cond, msg) => { + console.log(` ${cond ? 'ok' : 'FAIL'}: ${msg}`) + if (!cond) failed += 1 +} + +const root = mkdtempSync(join(tmpdir(), 'ot-grants-')) +// A symlink need not be creatable (Windows without developer mode): the cases +// that need one say so instead of failing the suite. +let canSymlink = true +const symlink = (target, path, kind) => { + if (!canSymlink) return false + try { + symlinkSync(target, path, kind) + return true + } catch { + canSymlink = false + console.log(` skip: symlinks are not creatable here (${path})`) + return false + } +} + +try { + const picked = join(root, 'picked') + const other = join(root, 'other') + mkdirSync(picked) + mkdirSync(other) + const pickedDir = realpathSync(picked) + const otherDir = realpathSync(other) + const subDir = join(pickedDir, 'sub') + mkdirSync(subDir) + + const pickedFile = join(pickedDir, 'id_ed25519') + writeFileSync(pickedFile, 'key') + const otherFile = join(otherDir, 'secret.txt') + writeFileSync(otherFile, 'secret') + + console.log('nothing is allowed before any grant') + ok(grantedPaths.readSource(pickedFile) === null, 'an un-granted existing file is refused') + ok(grantedPaths.readDirectory(pickedDir) === null, 'an un-granted directory is refused') + ok(grantedPaths.writeTarget(pickedDir, 'a.txt') === null, 'a write into it is refused') + + console.log('a picked file is readable, and only that file') + grantPickedFiles([pickedFile]) + ok(grantedPaths.readSource(pickedFile) === realpathSync(pickedFile), 'the granted file is accepted') + ok(grantedPaths.readSource(otherFile) === null, 'a different file is still refused') + ok(grantedPaths.readSource(pickedDir) === null, 'a directory is not a valid read source') + ok(grantedPaths.readSource(`${pickedFile}.nope`) === null, 'a missing path is refused') + ok(grantedPaths.readSource('') === null, 'an empty path is refused') + + console.log('a picked directory accepts writes below it, and nothing else') + grantPickedDirectory(pickedDir) + ok(grantedPaths.readDirectory(pickedDir) === pickedDir, 'the granted directory is accepted') + ok(grantedPaths.readDirectory(subDir) === subDir, 'a subdirectory of it is accepted') + ok(grantedPaths.readDirectory(otherDir) === null, 'an unrelated directory is refused') + ok( + grantedPaths.writeTarget(pickedDir, 'new.txt') === join(pickedDir, 'new.txt'), + 'a fresh leaf lands in the directory' + ) + ok( + grantedPaths.writeTarget(subDir, 'new.txt') === join(subDir, 'new.txt'), + 'a fresh leaf lands in a subdirectory' + ) + ok(grantedPaths.writeTarget(otherDir, 'new.txt') === null, 'an unrelated directory is refused') + ok( + grantedPaths.writeTarget(pickedDir, 'new.txt') === join(pickedDir, 'new.txt'), + 'extra arguments do not change the target' + ) + + console.log('a peer-supplied name cannot climb out of the directory') + for (const name of ['../escape.txt', '..\\escape.txt', '..', '.', '', 'a/b', 'a\\b', null, 42, {}]) { + ok(grantedPaths.writeTarget(pickedDir, name) === null, `refused: ${JSON.stringify(name) ?? name}`) + } + + console.log('a symlink at the target name is resolved, not followed blindly') + const outside = join(otherDir, 'outside.txt') + writeFileSync(outside, 'orig') + if (symlink(outside, join(pickedDir, 'escape-link.txt'), 'file')) { + ok( + grantedPaths.writeTarget(pickedDir, 'escape-link.txt') === null, + 'a link pointing outside the granted tree is refused' + ) + } + const insideTarget = join(subDir, 'real.txt') + if (symlink(insideTarget, join(pickedDir, 'inside-link.txt'), 'file')) { + ok( + grantedPaths.writeTarget(pickedDir, 'inside-link.txt') === join(pickedDir, 'inside-link.txt'), + 'a link pointing inside the granted tree is still accepted' + ) + } + const linkedDir = join(otherDir, 'linked') + if (symlink(linkedDir, join(pickedDir, 'linked-dir'), 'dir')) { + // Nothing was ever granted at other/linked, so the resolved path is outside. + ok( + grantedPaths.writeTarget(join(pickedDir, 'linked-dir'), 'x.txt') === null, + 'a symlinked directory leading outside is refused' + ) + } + + console.log('the wrong dialog grants nothing') + grantPickedFiles([otherDir]) + ok(grantedPaths.readSource(otherDir) === null, 'pickFiles of a directory grants no file') + grantPickedDirectory(pickedFile) + ok(grantedPaths.readDirectory(pickedFile) === null, 'pickDirectory of a file grants no directory') + + console.log('a grant is re-validated on every use') + const doomed = join(root, 'doomed') + mkdirSync(doomed) + grantPickedDirectory(doomed) + ok(grantedPaths.readDirectory(doomed) === realpathSync(doomed), 'usable while it exists') + rmSync(doomed, { recursive: true }) + ok(grantedPaths.readDirectory(doomed) === null, 'refused once it is gone') + ok(grantedPaths.writeTarget(doomed, 'a.txt') === null, 'and no write targets it') + + console.log('the grant API tolerates junk from its caller') + grantPickedFiles(undefined) + grantPickedFiles('not-an-array') + grantPickedFiles([null, 42, {}]) + grantPickedDirectory(undefined) + grantPickedDirectory('') + ok(grantedPaths.readSource(null) === null, 'a null source is refused') + ok(grantedPaths.readDirectory(undefined) === null, 'an undefined directory is refused') + ok(true, 'no junk input threw') +} finally { + rmSync(root, { recursive: true, force: true }) +} + +if (failed > 0) { + console.error(`\n[local-path-grants] ${failed} check(s) FAILED`) + process.exit(1) +} +console.log('\n[local-path-grants] ALL CHECKS PASSED') diff --git a/tests/sftp-real.mjs b/tests/sftp-real.mjs index 7e36ec1..6947d5e 100644 --- a/tests/sftp-real.mjs +++ b/tests/sftp-real.mjs @@ -5,11 +5,29 @@ import { createRequire } from 'module' import { randomUUID } from 'crypto' import { promises as fsp } from 'fs' +import { join } from 'path' const require_ = createRequire(import.meta.url) const sessionLayer = require_('./.session-e2e.cjs') const sftpMod = await import('./.sftp-svc.mjs') sftpMod.registerSftpClientProvider((id) => sessionLayer.getSshClient(id)) +/** + * Local-path admission. In the app the default policy only accepts paths the + * user granted through a native dialog (localPathGrants.ts); this harness picks + * its own temp paths and has no dialog to grant from, so it supplies a + * permissive double. It is NOT a re-export of the real policy — the admission + * rules have their own test (tests/local-path-grants.mjs) and these scenarios + * stay about transfer mechanics. + */ +sftpMod.setLocalPathPolicy({ + readSource: (p) => (typeof p === 'string' ? p : null), + readDirectory: (d) => (typeof d === 'string' && d !== '' ? d : null), + writeTarget: (dir, name) => + typeof dir === 'string' && dir !== '' && typeof name === 'string' && name !== '' + ? join(dir, name) + : null +}) + const events = [] sessionLayer.configureSessionRuntime({ broadcast: (channel, payload) => events.push({ channel, payload }), diff --git a/tests/zmodem-e2e.mjs b/tests/zmodem-e2e.mjs index 35c8c36..a39886e 100644 --- a/tests/zmodem-e2e.mjs +++ b/tests/zmodem-e2e.mjs @@ -83,6 +83,23 @@ if (!existsSync(bundlePath)) { const zmodem = require_('zmodem.js') const engine = require_(bundlePath) +/** + * Local-path admission for the engine. In the app the default policy only + * accepts paths the user granted through a native dialog (localPathGrants.ts), + * and this harness has no dialog to grant from — so the scenarios supply their + * own double. It is deliberately permissive (and deliberately NOT a re-export + * of the real policy, which is what keeps these scenarios about transfer + * mechanics rather than admission rules). + */ +engine.setLocalPathPolicy({ + readSource: (p) => (typeof p === 'string' ? p : null), + readDirectory: (d) => (typeof d === 'string' && d !== '' ? d : null), + writeTarget: (dir, name) => + typeof dir === 'string' && dir !== '' && typeof name === 'string' && name !== '' + ? join(dir, name) + : null +}) + const sleep = (ms) => new Promise((r) => setTimeout(r, ms)) /**