From a817131110e20580cbc9882a79472c124294fa5e Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Sun, 30 Aug 2026 13:56:41 +0000 Subject: [PATCH] feat: sync only the files and folders you picked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both surfaces could sync a directory. Neither could sync three things out of it, which is the one thing an SFTP client makes trivial. Desktop: tick entries in the source pane and the transfer button changes from "Sync to web-01" to "Send 2 -> web-01". Only those entries move. Nothing ticked keeps the old meaning, the whole directory. The panes already tracked a selection; it just had nothing to do with transfers. CLI: `--only NAME`, repeatable. diskpush ./site/ prod:/var/www/ --only index.html --only assets diskpush mirror ./cache/ prod:/srv/cache/ --only sessions The names go to rsync as a NUL-separated `--files-from` list rather than as extra source arguments. Both work; the list is not bounded by the command-line length limit, so four hundred files is the same shape as one, and with `--from0` it can express every name a filesystem allows — including the ones containing a newline, which no line-based list can. Two behaviours of `--files-from`, both established against rsync 3.4.1 rather than read off the flag summary, and both of which would have shipped as bugs: - It turns recursion OFF, and `--archive` does not turn it back on. A selected folder arrives as an EMPTY DIRECTORY. `buildRsyncArgs` now restates `--recursive` whenever a files-from list is in play. - `--delete` stays scoped to the listed entries. `mirror --only cache` removes destination files inside `cache/` and leaves the rest of the destination alone — the behaviour you would want, and worth stating because it is not the one you would fear. A selection is a choice among what the source directory holds, so a name is the only thing it can be: `writeSelectionList` refuses `..`, absolute paths and NUL bytes, and the IPC contract types the field as entry names, so a compromised renderer cannot turn a selection into a path. Also moves `EntryNameSchema` above its first use. `TransferRequestSchema` now references it, and a `const` is not hoisted — declaring it afterwards is a temporal-dead-zone ReferenceError at import time, which typechecks perfectly and kills the main process on launch. Caught by loading the built module, not by reading it. Verified against real files end to end: one file; a folder, which must arrive with its contents; several at once including a name with spaces; `--dry-run` changing nothing; `..` and absolute paths refused; no temp list left behind; and `mirror --only` leaving an unrelated destination file untouched. The desktop change was screenshotted — two rows ticked, "2 selected" in the pane footer, "SEND 2 web-01" on the armed button and "SYNC TO Local" on the other, which is right because the count belongs to the source pane. 504 tests, up from 491. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UeSWg1Czsb2Lwxj8vHUnA4 --- README.md | 2 + apps/cli/src/commands/transfer-helpers.ts | 3 + apps/cli/src/commands/transfer.ts | 63 +++++++++- apps/cli/src/parse-argv.ts | 1 + .../electron/main/services/transfers.ts | 41 ++++++- apps/desktop/electron/shared/contract.ts | 44 ++++--- apps/desktop/src/app/page.tsx | 5 + apps/desktop/src/components/transfer-rail.tsx | 18 ++- docs/cli.md | 31 +++++ docs/desktop.md | 13 ++ packages/rsync-core/src/args.ts | 13 +- packages/rsync-core/src/index.ts | 1 + packages/rsync-core/src/selection.test.ts | 113 ++++++++++++++++++ packages/rsync-core/src/selection.ts | 100 ++++++++++++++++ packages/schemas/src/rsync-options.ts | 9 ++ 15 files changed, 433 insertions(+), 24 deletions(-) create mode 100644 packages/rsync-core/src/selection.test.ts create mode 100644 packages/rsync-core/src/selection.ts diff --git a/README.md b/README.md index d7fb4a5..c5fe9de 100644 --- a/README.md +++ b/README.md @@ -14,6 +14,8 @@ diskpush ./data/ prod:/data/ -- --checksum # your own rsync flags ``` - **Local → server**, **server → local**, and **server → server** directly. +- **Sync a whole directory, or just what you picked.** Tick files and folders + in the pane, or `--only NAME` from the CLI. - **Archive metadata by default.** Permissions, timestamps, symlinks. - **Resumable by default.** An interrupted transfer keeps its partial data. - **Skips unchanged files.** Re-running a job moves almost nothing. diff --git a/apps/cli/src/commands/transfer-helpers.ts b/apps/cli/src/commands/transfer-helpers.ts index 58fe29e..6d43862 100644 --- a/apps/cli/src/commands/transfer-helpers.ts +++ b/apps/cli/src/commands/transfer-helpers.ts @@ -8,7 +8,10 @@ export { planTransfer, runPlan, runToCompletion, + writeSelectionList, + describeSelection, type ExecutionPlan, + type SelectionList, } from '@diskpush/rsync-core' import { runToCompletion, type ExecutionPlan } from '@diskpush/rsync-core' diff --git a/apps/cli/src/commands/transfer.ts b/apps/cli/src/commands/transfer.ts index 0e7aabb..abda98e 100644 --- a/apps/cli/src/commands/transfer.ts +++ b/apps/cli/src/commands/transfer.ts @@ -7,15 +7,18 @@ import { intersectCapabilities, planTransfer, runPlan, + writeSelectionList, + describeSelection, summarizeChangesFrom, type ExecutionPlan, + type SelectionList, } from './transfer-helpers.js' import type { Change, RsyncOptions } from '@diskpush/schemas' import { summarizeChanges, topologyOf } from '@diskpush/schemas' import { EXIT } from '../exit-codes.js' import { estimateRemaining, formatBytes, formatDuration, formatRate, pluralize, table } from '../format.js' import { failure, type Output } from '../output.js' -import { flagValue, hasFlag, type ParsedArgv } from '../parse-argv.js' +import { flagValue, flagValues, hasFlag, type ParsedArgv } from '../parse-argv.js' import { detectLocalCapabilities, optionsFromFlags, resolveEndpoint } from '../resolve.js' import type { RsyncCapabilities } from '@diskpush/rsync-core' @@ -61,6 +64,61 @@ export async function runTransfer( if (alias.deleteMode !== 'off') options.deleteMode = alias.deleteMode + /* + * `--only NAME` transfers just the entries named, rather than everything in + * the source directory — the thing an SFTP client makes trivial and a bare + * rsync does not. + * + * The names go to rsync as a NUL-separated `--files-from` list, which is not + * bounded by the command-line length limit and can express any name a + * filesystem allows. `writeSelectionList` refuses `..` and absolute paths: + * a selection is a choice among what the source directory holds, so a name + * is the only thing it can be. + */ + const only = flagValues(parsed, '--only') + let selection: SelectionList | null = null + if (only.length > 0) { + if (options.filesFrom) { + return failure(output, '--only and --files-from both choose what to send; use one.', EXIT.usage) + } + try { + selection = writeSelectionList(only) + } catch (error) { + return failure(output, (error as Error).message, EXIT.usage) + } + options.filesFrom = selection.path + options.from0 = true + } + + try { + return await runResolvedTransfer( + command, + parsed, + store, + output, + alias, + sourceInput, + destinationInput, + options, + only, + ) + } finally { + // rsync reads the list at startup, but it is not gone until the run is. + selection?.cleanup() + } +} + +async function runResolvedTransfer( + command: string, + parsed: ParsedArgv, + store: DiskPushStore, + output: Output, + alias: (typeof TRANSFER_ALIASES)[string], + sourceInput: string, + destinationInput: string, + options: RsyncOptions, + only: readonly string[], +): Promise { const source = await resolveEndpoint(store, sourceInput) const destination = await resolveEndpoint(store, destinationInput) const topology = topologyOf(source.endpoint, destination.endpoint) @@ -180,6 +238,9 @@ export async function runTransfer( output.line(`DiskPush: ${alias.label} ${describe(sourceInput)} -> ${describe(destinationInput)}`) output.line(`Source: ${sourceInput}`) output.line(`Destination: ${destinationInput}`) + // Named before the transfer runs, for the same reason a mirror shows its + // delete list: what is about to move is worth stating. + if (only.length > 0) output.line(`Only: ${describeSelection(only)}`) if (topology === 'remote-to-remote') { output.line('') output.line(`Direct path: ${source.endpoint.type === 'ssh' ? source.endpoint.host : '?'} -> ${destination.endpoint.type === 'ssh' ? destination.endpoint.host : '?'}`) diff --git a/apps/cli/src/parse-argv.ts b/apps/cli/src/parse-argv.ts index 0e1bd29..03c2579 100644 --- a/apps/cli/src/parse-argv.ts +++ b/apps/cli/src/parse-argv.ts @@ -33,6 +33,7 @@ export const VALUE_FLAGS = new Set([ '--exclude-from', '--include-from', '--files-from', + '--only', '--bwlimit', '--max-size', '--min-size', diff --git a/apps/desktop/electron/main/services/transfers.ts b/apps/desktop/electron/main/services/transfers.ts index df86f66..485c9e5 100644 --- a/apps/desktop/electron/main/services/transfers.ts +++ b/apps/desktop/electron/main/services/transfers.ts @@ -2,6 +2,8 @@ import { randomUUID } from 'node:crypto' import type { WebContents } from 'electron' import { intersectCapabilities, + writeSelectionList, + type SelectionList, parseRsyncCapabilities, planTransfer, runPlan, @@ -102,11 +104,32 @@ function optionsFrom(input: TransferOptions): RsyncOptions { }) } -async function buildPlan(request: TransferRequest, overrides: Partial = {}): Promise { +/** + * Turns the renderer's selection into a `--files-from` list. + * + * Returns null when nothing is selected, which means the whole directory — + * the behaviour the two-pane view has always had. The caller removes the list + * once rsync has exited. + */ +function selectionFor(request: TransferRequest): SelectionList | null { + return request.selection.length > 0 ? writeSelectionList(request.selection) : null +} + +async function buildPlan( + request: TransferRequest, + overrides: Partial = {}, + selection: SelectionList | null = null, +): Promise { const source = await resolveEndpoint(request.source) const destination = await resolveEndpoint(request.destination) const capabilities = await capabilitiesFor([source.connectionId, destination.connectionId]) const options = { ...optionsFrom(request.options), ...overrides } + if (selection) { + options.filesFrom = selection.path + // NUL-separated: a newline is legal in a filename, so a newline-separated + // list cannot express every name a directory can hold. + options.from0 = true + } const isServerToServer = source.endpoint.type === 'ssh' && destination.endpoint.type === 'ssh' const sourceConnection = source.connectionId ? await resolveConnection(source.connectionId) : null @@ -142,7 +165,15 @@ export type PreviewResult = { /** The dry run behind Preview Changes and behind every mirror. */ export async function previewTransfer(request: TransferRequest): Promise { - const plan = await buildPlan(request, { dryRun: true }) + const selection = selectionFor(request) + try { + return await previewWithPlan(await buildPlan(request, { dryRun: true }, selection)) + } finally { + selection?.cleanup() + } +} + +async function previewWithPlan(plan: ExecutionPlan): Promise { const result = await runToCompletion(plan) return { changes: result.changes, @@ -159,7 +190,10 @@ export async function previewTransfer(request: TransferRequest): Promise { - const plan = await buildPlan(request) + // rsync reads the list at startup, but the run owns it until it exits: the + // cleanup below is in the event loop's `finally`, not this function's. + const selection = selectionFor(request) + const plan = await buildPlan(request, {}, selection) const jobId = randomUUID() const db = await store() @@ -221,6 +255,7 @@ export async function startTransfer(request: TransferRequest, sender: WebContent } } running.delete(jobId) + selection?.cleanup() })() return { jobId, command: plan.display, control: plan.controlDisplay ?? null, warnings: plan.warnings } diff --git a/apps/desktop/electron/shared/contract.ts b/apps/desktop/electron/shared/contract.ts index a4b4279..4d5be0c 100644 --- a/apps/desktop/electron/shared/contract.ts +++ b/apps/desktop/electron/shared/contract.ts @@ -113,12 +113,39 @@ export const TransferOptionsSchema = z.object({ }) export type TransferOptions = z.infer +/** + * A single entry name inside a directory — never a path. + * + * Every mutating operation takes a directory plus one of these and joins them + * in the main process, so the renderer cannot walk out of the folder it is + * showing. `..`, a separator or a NUL would each be a way to do exactly that. + */ +export const EntryNameSchema = z + .string() + .min(1) + .max(255) + .refine((name) => !name.includes('/') && !name.includes('\\') && !name.includes('\0'), { + message: 'A name cannot contain a path separator.', + }) + .refine((name) => name !== '.' && name !== '..', { message: 'That name is reserved.' }) + .refine((name) => name.trim() === name, { message: 'A name cannot begin or end with a space.' }) + export const TransferRequestSchema = z.object({ source: EndpointRefSchema, destination: EndpointRefSchema, options: TransferOptionsSchema, /** Only meaningful for a delete-enabled job, and only after a preview. */ deletesConfirmed: z.boolean().default(false), + /** + * Send only these entries, rather than everything in the source directory. + * + * Entry names, not paths: each is one item the source pane is showing. The + * main process turns them into a `--files-from` list; the renderer never + * builds a path, so a selection cannot address anything the pane is not + * already looking at. Empty means the whole directory, which is what the + * two-pane view has always done. + */ + selection: z.array(EntryNameSchema).max(10_000).default([]), }) export type TransferRequest = z.infer @@ -135,23 +162,6 @@ export const RenameRequestSchema = z.object({ to: PathSchema, }) -/** - * A single entry name inside a directory — never a path. - * - * Every mutating operation takes a directory plus one of these and joins them - * in the main process, so the renderer cannot walk out of the folder it is - * showing. `..`, a separator or a NUL would each be a way to do exactly that. - */ -export const EntryNameSchema = z - .string() - .min(1) - .max(255) - .refine((name) => !name.includes('/') && !name.includes('\\') && !name.includes('\0'), { - message: 'A name cannot contain a path separator.', - }) - .refine((name) => name !== '.' && name !== '..', { message: 'That name is reserved.' }) - .refine((name) => name.trim() === name, { message: 'A name cannot begin or end with a space.' }) - /** Create a directory or an empty file: `name` inside `directory`. */ export const CreateEntryRequestSchema = z.object({ connectionId: ConnectionIdSchema.optional(), diff --git a/apps/desktop/src/app/page.tsx b/apps/desktop/src/app/page.tsx index fdefa83..26a199b 100644 --- a/apps/desktop/src/app/page.tsx +++ b/apps/desktop/src/app/page.tsx @@ -191,6 +191,10 @@ export default function Workspace() { destination: refFor(destination, withTrailingSlash(destination.path)), options: { deleteMode: mirror ? ('delay' as const) : ('off' as const) }, deletesConfirmed: false, + // Ticked entries in the source pane mean "just these", the way an SFTP + // client behaves. Nothing ticked keeps the old meaning: the whole + // directory. + selection: [...source.selected], }), [source, destination, mirror], ) @@ -409,6 +413,7 @@ export default function Workspace() { /> void }) { const Arrow = side === 'right' ? ArrowRight : ArrowLeft @@ -60,11 +63,17 @@ function Direction({ armed ? 'text-primary-foreground/70' : 'text-faint', )} > - Sync to + {armed && selectedCount > 0 ? `Send ${selectedCount}` : 'Sync to'} {label} - Copy into {label}, overwriting what differs + + {/* Only on the armed button: the count is the *source* pane's, and + the other button would make the other pane the source. */} + {armed && selectedCount > 0 + ? `Copy the ${selectedCount} selected ${selectedCount === 1 ? 'entry' : 'entries'} into ${label}` + : `Copy into ${label}, overwriting what differs`} + ) } @@ -118,6 +127,7 @@ export function TransferRail({ direction, mirror, busy, + selectedCount, leftLabel, rightLabel, onDirection, @@ -128,6 +138,8 @@ export function TransferRail({ direction: 'ltr' | 'rtl' mirror: boolean busy: boolean + /** Ticked entries in the source pane. 0 means the whole directory. */ + selectedCount: number leftLabel: string rightLabel: string onDirection: (direction: 'ltr' | 'rtl') => void @@ -143,6 +155,7 @@ export function TransferRail({
{ + it('accepts an ordinary entry name', () => { + expect(() => assertSelectable('one.txt')).not.toThrow() + expect(() => assertSelectable('a folder')).not.toThrow() + expect(() => assertSelectable('nested/deep.txt')).not.toThrow() + }) + + it('refuses anything that climbs out of the source directory', () => { + expect(() => assertSelectable('..')).toThrow(SelectionError) + expect(() => assertSelectable('../etc/passwd')).toThrow(/climb out/) + expect(() => assertSelectable('a/../../b')).toThrow(/climb out/) + }) + + it('refuses an absolute path, which would ignore the source entirely', () => { + expect(() => assertSelectable('/etc/passwd')).toThrow(/relative to the source/) + }) + + it('refuses names that are not names', () => { + expect(() => assertSelectable('')).toThrow(SelectionError) + expect(() => assertSelectable('.')).toThrow(SelectionError) + expect(() => assertSelectable('a//b')).toThrow(SelectionError) + expect(() => assertSelectable('a\0b')).toThrow(/NUL/) + }) +}) + +describe('writeSelectionList', () => { + it('writes a NUL-separated, NUL-terminated list', () => { + const list = writeSelectionList(['one.txt', 'keep']) + try { + expect(readFileSync(list.path, 'utf8')).toBe('one.txt\0keep\0') + } finally { + list.cleanup() + } + }) + + it('can express a name containing a newline, which a line-based list cannot', () => { + // rsync splits such a name across two entries in a newline-separated list + // and fails both halves with "No such file or directory". + const list = writeSelectionList(['new\nline.txt']) + try { + expect(readFileSync(list.path, 'utf8')).toBe('new\nline.txt\0') + } finally { + list.cleanup() + } + }) + + it('removes the list, and does not mind being told twice', () => { + const list = writeSelectionList(['a']) + expect(existsSync(list.path)).toBe(true) + list.cleanup() + expect(existsSync(list.path)).toBe(false) + expect(() => list.cleanup()).not.toThrow() + }) + + it('refuses an empty selection rather than silently sending everything', () => { + expect(() => writeSelectionList([])).toThrow(/Nothing was selected/) + }) + + it('validates every name, not just the first', () => { + expect(() => writeSelectionList(['fine.txt', '../escape'])).toThrow(/climb out/) + }) +}) + +describe('buildRsyncArgs with a selection', () => { + const endpoints = { + source: { type: 'local' as const, path: '/src/' }, + destination: { type: 'local' as const, path: '/dst/' }, + } + + it('restates --recursive, which --files-from turns off', () => { + /* + * Verified against rsync 3.4.1: with `-a --files-from` a selected folder + * arrives as an EMPTY directory, and `--archive` does not save you. Only + * an explicit `-r` brings its contents. Without this the feature looks + * like it works and silently transfers nothing inside a folder. + */ + const built = buildRsyncArgs({ + ...endpoints, + options: { ...defaultRsyncOptions(), filesFrom: '/tmp/list', from0: true }, + }) + expect(built.args).toContain('--recursive') + expect(built.args).toContain('--from0') + expect(built.args).toContain('--files-from=/tmp/list') + }) + + it('does not add --from0 unless asked, so a hand-written list still works', () => { + const built = buildRsyncArgs({ + ...endpoints, + options: { ...defaultRsyncOptions(), filesFrom: '/tmp/list' }, + }) + expect(built.args).not.toContain('--from0') + expect(built.args).toContain('--files-from=/tmp/list') + }) + + it('leaves an ordinary transfer alone', () => { + const built = buildRsyncArgs({ ...endpoints, options: defaultRsyncOptions() }) + expect(built.args).not.toContain('--from0') + expect(built.args.some((arg) => arg.startsWith('--files-from'))).toBe(false) + }) +}) + +describe('describeSelection', () => { + it('names a single entry, and counts the rest', () => { + expect(describeSelection(['one.txt'])).toBe('one.txt') + expect(describeSelection(['a', 'b', 'c'])).toBe('3 selected entries') + }) +}) diff --git a/packages/rsync-core/src/selection.ts b/packages/rsync-core/src/selection.ts new file mode 100644 index 0000000..96b407f --- /dev/null +++ b/packages/rsync-core/src/selection.ts @@ -0,0 +1,100 @@ +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +/** + * Transferring only the entries someone picked, rather than a whole directory. + * + * The selection is a list of names relative to the source directory, and it + * reaches rsync as a `--files-from` list rather than as extra source + * arguments. Both work; the list wins on two counts. It is not bounded by the + * command-line length limit, so selecting four hundred files is the same shape + * as selecting one. And with `--from0` it can express every name a filesystem + * allows, including the ones with a newline in them, which no argument list + * assembled from a newline-separated source can. + * + * Two behaviours of `--files-from` are worth knowing, both verified against + * rsync 3.4.1 rather than taken from the manual: + * + * - It turns recursion **off**, and `--archive` does not turn it back on. A + * selected folder arrives as an empty directory unless `--recursive` is + * restated. `buildRsyncArgs` restates it. + * - `--delete` stays scoped to the listed entries. Mirroring a selection + * removes destination files inside those entries and leaves the rest of the + * destination alone, which is the behaviour you would want and not the one + * you would fear. + */ + +export class SelectionError extends Error { + constructor(message: string) { + super(message) + this.name = 'SelectionError' + } +} + +/** + * Rejects anything that is not a plain entry name inside the source directory. + * + * A selection comes from a file listing, so a name is all it should ever be. + * `..` would reach outside the directory being transferred, and an absolute + * path would ignore it entirely — neither is a thing a selection can mean. + */ +export function assertSelectable(name: string): void { + if (name.length === 0) throw new SelectionError('An empty name cannot be selected.') + if (name.startsWith('/')) throw new SelectionError(`A selection must be relative to the source: ${JSON.stringify(name)}.`) + if (name.includes('\0')) throw new SelectionError('A name cannot contain a NUL byte.') + const segments = name.split('/') + if (segments.some((segment) => segment === '..')) { + throw new SelectionError(`A selection cannot climb out of the source directory: ${JSON.stringify(name)}.`) + } + if (segments.some((segment) => segment === '.' || segment === '')) { + throw new SelectionError(`${JSON.stringify(name)} is not a usable entry name.`) + } +} + +export type SelectionList = { + /** Path to the NUL-separated list, for `--files-from`. */ + path: string + /** Removes the list and its directory. Safe to call more than once. */ + cleanup: () => void +} + +/** + * Writes a selection to a NUL-separated list file in a private temp directory. + * + * The caller removes it with `cleanup()` once rsync has exited — rsync reads + * the list at startup, but deleting it out from under a run that has not begun + * would be a race, so it is not tied to anything cleverer than the caller's + * `finally`. + */ +export function writeSelectionList(names: readonly string[]): SelectionList { + if (names.length === 0) throw new SelectionError('Nothing was selected.') + for (const name of names) assertSelectable(name) + + const directory = mkdtempSync(join(tmpdir(), 'diskpush-selection-')) + const path = join(directory, 'files-from') + // NUL-separated, with a trailing NUL: `--from0` reads it as a list of + // NUL-terminated names. + writeFileSync(path, `${names.join('\0')}\0`, { mode: 0o600 }) + + let removed = false + return { + path, + cleanup: () => { + if (removed) return + removed = true + try { + rmSync(directory, { recursive: true, force: true }) + } catch { + // A temp file that outlives the run is untidy, not a failure of the + // transfer, and the transfer's outcome is what the caller reports. + } + }, + } +} + +/** One line describing the selection, for a preview or a summary. */ +export function describeSelection(names: readonly string[]): string { + if (names.length === 1) return names[0]! + return `${names.length} selected entries` +} diff --git a/packages/schemas/src/rsync-options.ts b/packages/schemas/src/rsync-options.ts index 7f6aff6..f878514 100644 --- a/packages/schemas/src/rsync-options.ts +++ b/packages/schemas/src/rsync-options.ts @@ -52,6 +52,15 @@ export const RsyncOptionsSchema = z.object({ excludeFrom: z.string().min(1).nullable().default(null), includeFrom: z.string().min(1).nullable().default(null), filesFrom: z.string().min(1).nullable().default(null), + /** + * Read `--files-from` as NUL-separated rather than newline-separated. + * + * A newline is legal in a filename, so a newline-separated list cannot + * express every name a directory can hold — rsync splits such a name in two + * and fails both halves with "No such file or directory". Anything DiskPush + * generates uses this. + */ + from0: z.boolean().default(false), maxSize: z.string().min(1).nullable().default(null), minSize: z.string().min(1).nullable().default(null), pruneEmptyDirs: z.boolean().default(false),