diff --git a/README.md b/README.md index 21698fb..4bee9c2 100644 --- a/README.md +++ b/README.md @@ -142,6 +142,31 @@ Saved connections live in a plain JSON file in the OS app-data directory (On Macs, function keys may need `fn` — every action also has a toolbar button.) +## Same-file protection + +Copying a file or folder onto itself would truncate it to zero bytes (the +destination is opened for writing before the source is read). BunSCP blocks +this at several layers: + +- **Same session / same host**: every connection carries a filesystem identity + (`local`, or `resolvedIP:port` — the hostname is DNS-resolved, so + `myserver.lan` and its IP compare equal even across two panels/tabs holding + separate SSH sessions to the same server). Equal identity + equal or nested + paths ⇒ the transfer is refused before anything is opened. +- **Hidden aliases** (two *different* addresses that reach the same machine — + NAT, split DNS): when identities differ but source and destination paths + collide anyway, a probe file is written through one connection and looked up + through the other; if it's visible on both, they're the same machine and the + copy is refused. (SFTP offers no device/inode identity — the mechanism + `cp`/`rsync` use locally — so probing is the only deterministic test.) +- The overwrite dialog never offers to "Replace" a file with itself — such + items are auto-skipped with a notice. + +**Known residual limitation**: the same filesystem mounted at *different +paths* on two connections (bind mounts, NFS exported twice, shared container +volumes) cannot be detected — the paths differ, so no guard or probe triggers. +Be careful when two panels reach the same data through different mount points. + ## Portability notes The codebase is platform-neutral except for two seams, both macOS-only today: diff --git a/src/bun/fs/local.ts b/src/bun/fs/local.ts index 85524c2..9f96b8b 100644 --- a/src/bun/fs/local.ts +++ b/src/bun/fs/local.ts @@ -24,6 +24,7 @@ function toEntry(name: string, full: string, st: Stats): FileEntry { export class LocalProvider implements FileSystemProvider { readonly kind = "local" as const; + readonly fsKey = "local"; join = (...parts: string[]) => path.join(...parts); dirname = (p: string) => path.dirname(p); diff --git a/src/bun/fs/provider.ts b/src/bun/fs/provider.ts index 552c1ca..e24c6ee 100644 --- a/src/bun/fs/provider.ts +++ b/src/bun/fs/provider.ts @@ -12,6 +12,15 @@ import type { FileEntry } from "../../shared/types"; */ export interface FileSystemProvider { readonly kind: "local" | "sftp"; + /** + * Identity of the FILESYSTEM this provider reaches, independent of the + * session: "local", or ":" for SFTP. Two panels connected + * to the same server hold two SSH sessions but share one fsKey — self-copy + * guards key on this, never on sessionId (a sessionId comparison caused a + * truncate-to-zero data-loss bug). Username is deliberately excluded: the + * same path on the same host:port is the same file regardless of login. + */ + readonly fsKey: string; join(...parts: string[]): string; dirname(p: string): string; basename(p: string): string; diff --git a/src/bun/fs/sftp.ts b/src/bun/fs/sftp.ts index 10f12fe..a45287a 100644 --- a/src/bun/fs/sftp.ts +++ b/src/bun/fs/sftp.ts @@ -1,3 +1,4 @@ +import dns from "node:dns/promises"; import { readFileSync } from "node:fs"; import os from "node:os"; import path from "node:path"; @@ -24,12 +25,16 @@ function expandTilde(p: string): string { export class SftpProvider implements FileSystemProvider { readonly kind = "sftp" as const; + readonly fsKey: string; private closedByUs = false; private constructor( private client: Client, private sftp: SFTPWrapper, - ) {} + fsKey: string, + ) { + this.fsKey = fsKey; + } /** Resolves once the SSH connection is ready and the SFTP subsystem is open. */ static async connect( @@ -51,6 +56,16 @@ export class SftpProvider implements FileSystemProvider { if (config.auth.passphrase) connectConfig.passphrase = config.auth.passphrase; } + // Filesystem identity: resolve the host so a hostname and its IP compare + // equal (self-copy guards). Falls back to the raw host string offline. + let resolvedHost = config.host.toLowerCase(); + try { + resolvedHost = (await dns.lookup(config.host)).address; + } catch { + // unresolvable (e.g. already an IP on some systems) — keep the string + } + const fsKey = `${resolvedHost}:${config.port}`; + const client = new Client(); const provider = await new Promise((resolve, reject) => { client @@ -60,7 +75,7 @@ export class SftpProvider implements FileSystemProvider { client.end(); reject(err); } else { - resolve(new SftpProvider(client, sftp)); + resolve(new SftpProvider(client, sftp, fsKey)); } }); }) diff --git a/src/bun/rpc-handlers.ts b/src/bun/rpc-handlers.ts index e8d0642..fcf367a 100644 --- a/src/bun/rpc-handlers.ts +++ b/src/bun/rpc-handlers.ts @@ -53,7 +53,7 @@ export function createAppRPC({ registry, engine, store, settings, edits, events sessionId = registry.add(provider); const homeDir = await provider.homeDir(); console.log(`[rpc] connected ${sessionId} -> ${config.username}@${config.host}`); - return { sessionId, homeDir }; + return { sessionId, homeDir, fsKey: provider.fsKey }; }), disconnect: ({ sessionId }) => diff --git a/src/bun/transfers/engine.ts b/src/bun/transfers/engine.ts index 3f52825..db9a0c0 100644 --- a/src/bun/transfers/engine.ts +++ b/src/bun/transfers/engine.ts @@ -181,6 +181,9 @@ export class TransferEngine { try { const src = this.registry.get(snap.srcSessionId); const dst = this.registry.get(snap.dstSessionId); + // same FILESYSTEM (never compare sessionIds — two panels on one server + // hold two sessions, and that comparison once truncated files to zero) + const sameFs = src.fsKey === dst.fsKey; snap.state = "scanning"; this.emit(job, true); @@ -189,7 +192,7 @@ export class TransferEngine { let files: { srcPath: string; dstPath: string; size: number; name: string }[]; if (job.type === "dir") { const dstRoot = dst.join(snap.dstDir, snap.name); - this.guardAgainstSelfCopy(snap, dstRoot); + await this.guardAgainstSelfCopy(snap, dstRoot, sameFs, src, dst, true); const tree = await walkTree(src, snap.srcPath); for (const segments of tree.dirs) { if (job.cancelled) throw new Error("transfer cancelled"); @@ -203,11 +206,21 @@ export class TransferEngine { })); } else { const dstPath = dst.join(snap.dstDir, snap.name); - this.guardAgainstSelfCopy(snap, dstPath); + await this.guardAgainstSelfCopy(snap, dstPath, sameFs, src, dst, false); const st = await src.stat(snap.srcPath); files = [{ srcPath: snap.srcPath, dstPath, size: st.size, name: snap.name }]; } + // belt & suspenders: an identical path must never reach the write + // stream (createWriteStream truncates the destination immediately) + if (sameFs) { + for (const file of files) { + if (file.srcPath === file.dstPath) { + throw new Error(`"${file.name}": source and destination are the same file`); + } + } + } + snap.totalFiles = files.length; snap.totalBytes = files.reduce((sum, f) => sum + f.size, 0); snap.state = "running"; @@ -281,14 +294,78 @@ export class TransferEngine { job.activeStreams.clear(); } - /** Copying onto itself truncates the source; into a descendant never ends. */ - private guardAgainstSelfCopy(snap: TransferSnapshot, dstPath: string): void { - if (snap.srcSessionId !== snap.dstSessionId) return; - if (dstPath === snap.srcPath) { - throw new Error("Source and destination are the same"); + /** + * Copying onto itself truncates the source; into a descendant never ends. + * Keyed on filesystem identity (fsKey), not sessionId. When the keys differ + * but the paths collide anyway, a probe file decides whether the two + * connections secretly reach the same machine (NAT / alias hosts). + */ + private async guardAgainstSelfCopy( + snap: TransferSnapshot, + dstPath: string, + sameFs: boolean, + src: FileSystemProvider, + dst: FileSystemProvider, + isDir: boolean, + ): Promise { + const samePath = dstPath === snap.srcPath; + const nestedPath = dstPath.startsWith( + snap.srcPath.endsWith("/") ? snap.srcPath : `${snap.srcPath}/`, + ); + if (!samePath && !nestedPath) return; + + // Paths collide. Same fsKey -> definitely the same file/folder. Different + // fsKey -> either a coincidence (two servers both have /root/x.conf) or a + // hidden alias of one machine; only a probe can tell (SFTP exposes no + // device/inode identity — the technique cp/rsync use locally). + const definitelySame = + sameFs || (await this.probeSameFilesystem(src, dst, dst.dirname(dstPath))); + if (!definitelySame) return; + + if (samePath) { + throw new Error( + isDir ? "Cannot copy a folder onto itself" : "Source and destination are the same file", + ); } - if (dstPath.startsWith(snap.srcPath.endsWith("/") ? snap.srcPath : `${snap.srcPath}/`)) { - throw new Error("Cannot copy a folder into itself"); + throw new Error("Cannot copy a folder into itself"); + } + + /** + * Deterministic same-filesystem test across two connections: write a token + * file via dst, look for it via src. Visible + matching content = one + * machine behind two addresses. Fails open (returns false) when the probe + * itself cannot run (e.g. destination not writable) — the copy then + * proceeds and permissions produce their own error. + */ + private async probeSameFilesystem( + src: FileSystemProvider, + dst: FileSystemProvider, + dir: string, + ): Promise { + const token = crypto.randomUUID(); + const probePath = dst.join(dir, `.bunscp-probe-${token.slice(0, 8)}`); + try { + await new Promise((resolve, reject) => { + const stream = dst.createWriteStream(probePath); + stream.on("error", reject); + stream.end(token, () => resolve()); + }); + try { + const chunks: Buffer[] = []; + for await (const chunk of src.createReadStream(src.join(dir, `.bunscp-probe-${token.slice(0, 8)}`))) { + chunks.push(typeof chunk === "string" ? Buffer.from(chunk) : chunk); + } + const seen = Buffer.concat(chunks).toString("utf8") === token; + if (seen) console.log(`[engine] probe: ${src.fsKey} and ${dst.fsKey} are the same machine`); + return seen; + } catch { + return false; // src cannot see the token -> different filesystems + } + } catch (error) { + console.log(`[engine] same-fs probe skipped: ${error instanceof Error ? error.message : error}`); + return false; + } finally { + await dst.deleteFile(probePath).catch(() => {}); } } diff --git a/src/shared/rpc.ts b/src/shared/rpc.ts index 90c0211..3c2ff18 100644 --- a/src/shared/rpc.ts +++ b/src/shared/rpc.ts @@ -22,7 +22,7 @@ export type AppRPC = { // sessions connect: { params: { config: ConnectionConfig }; - response: Result<{ sessionId: string; homeDir: string }>; + response: Result<{ sessionId: string; homeDir: string; fsKey: string }>; }; disconnect: { params: { sessionId: string }; response: Result }; // filesystem (uniform across "local" and sftp sessions) diff --git a/ui/src/app/panels.service.ts b/ui/src/app/panels.service.ts index 2563a68..d5785f1 100644 --- a/ui/src/app/panels.service.ts +++ b/ui/src/app/panels.service.ts @@ -13,6 +13,8 @@ interface PanelDeps { export class PanelState { readonly sessionId = signal('local'); + /** Filesystem identity ("local" or resolvedIp:port) — self-copy guards use this. */ + readonly fsKey = signal('local'); readonly title = signal('Local'); readonly connecting = signal(false); readonly cwd = signal(''); @@ -339,6 +341,7 @@ export class PanelsService { void this.rpc.request.disconnect({ sessionId: previous }); } panel.sessionId.set(res.sessionId); + panel.fsKey.set(res.fsKey); panel.title.set(title); await panel.load(config.initialPath ?? res.homeDir); return true; @@ -357,6 +360,7 @@ export class PanelsService { private resetToLocal(panel: PanelState): void { panel.sessionId.set('local'); + panel.fsKey.set('local'); panel.title.set('Local'); void panel.load(null); } diff --git a/ui/src/app/transfers.service.ts b/ui/src/app/transfers.service.ts index 993c93b..f5d1828 100644 --- a/ui/src/app/transfers.service.ts +++ b/ui/src/app/transfers.service.ts @@ -56,12 +56,13 @@ export class TransfersService { this.toast.info('Nothing to copy (symlinks are skipped)'); return; } - if (src.sessionId() === dst.sessionId() && src.cwd() === dst.cwd()) { + const sameFs = src.fsKey() === dst.fsKey(); + if (sameFs && src.cwd() === dst.cwd()) { this.toast.info('Both panels show the same folder'); return; } - const finalTargets = await this.resolveCollisions(targets, dst.entries()); + const finalTargets = await this.resolveCollisions(targets, dst.entries(), sameFs); if (finalTargets === null) return; // user cancelled if (!finalTargets.length) { this.toast.info('Nothing to copy — all conflicting items were skipped'); @@ -84,6 +85,7 @@ export class TransfersService { private async resolveCollisions( targets: FileEntry[], dstEntries: FileEntry[], + sameFs: boolean, ): Promise { const dstByName = new Map(dstEntries.map((e) => [e.name, e])); const collisions = targets.filter((t) => { @@ -101,6 +103,11 @@ export class TransfersService { finalTargets.push(target); continue; } + if (sameFs && dstEntry.path === target.path) { + // replacing a file with ITSELF would only destroy it — never offer that + this.toast.info(`Skipped "${target.name}" — it is the same file`); + continue; + } if ((dstEntry.type === 'dir') !== (target.type === 'dir')) { this.toast.info(`Skipped "${target.name}" — a different kind of item is in the way`); continue;