From 2763a633846d6fa6fec2824598d85a1de7ba2a06 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 14:51:31 +0000 Subject: [PATCH] Never delete through symlinked directories, add --no-follow-symlinks sync -d deleted every file under a symlinked directory in the local destination that was missing from the bucket, and mv deleted the real files behind a symlinked directory after uploading them. Files reached through a linked directory are now marked: sync -d keeps them and mv refuses them. cp, mv and sync take --no-follow-symlinks, as aws s3 does. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01NBZ61xoLMt7wvuViatMdxW --- README.md | 3 ++- src/commands/blob/cp.ts | 5 ++++- src/commands/blob/sync.ts | 5 +++-- src/commands/blob/transfer.ts | 33 ++++++++++++++++++++-------- tests/unit/blob-transfer.test.ts | 37 ++++++++++++++++++++++++++++++++ 5 files changed, 70 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 1263c35..2ca6e4a 100644 --- a/README.md +++ b/README.md @@ -113,7 +113,8 @@ order, last match wins), `-n/--dryrun`, `-d/--delete`, `--size-only`, `--exact-timestamps`, `--content-type`, `--cache-control`, `--metadata`, `--expected-size`, `--concurrency` and `-q/--quiet`. Copies between buckets reset Cache-Control to the default unless `--cache-control` is given. Local symbolic -links are followed. +links are followed unless `--no-follow-symlinks` is given. Nothing is deleted +through a link to a directory: `mv` refuses such files and `sync -d` keeps them. Progress goes to stderr and a JSON summary to stdout. Transfers retry transient failures, keep going past a failed file, and exit unsuccessfully at the end. Large diff --git a/src/commands/blob/cp.ts b/src/commands/blob/cp.ts index d0b2944..689a876 100644 --- a/src/commands/blob/cp.ts +++ b/src/commands/blob/cp.ts @@ -132,7 +132,7 @@ Examples: const [skip, sourceInside] = recursive ? await Promise.all([nestedPrefix(source, destination, resolver), nestedPrefix(destination, source, resolver)]) : []; - const entries = (await listSource(source, recursive, resolver)).filter((entry) => + const entries = (await listSource(source, recursive, resolver, options.followSymlinks)).filter((entry) => isIncluded(entry.rel, filters) && !(skip !== undefined && entry.location.type === "blob" && entry.location.key.startsWith(skip))); const into = recursive || (destination.type === "blob" ? !destination.key || destination.key.endsWith("/") @@ -144,6 +144,9 @@ Examples: // Zero-byte "folder" markers have no local file to become. if (destination.type === "local" && entry.rel.endsWith("/")) continue; try { + if (name === "mv" && entry.linked) { + throw new Error("is behind a symbolic link to a directory, so moving it would delete the link's target; use cp, or --no-follow-symlinks"); + } const target = destinationFor(entry, destination, into); // An mv folds case everywhere: on a case-insensitive Linux mount, two keys writing one // file would otherwise both be deleted. diff --git a/src/commands/blob/sync.ts b/src/commands/blob/sync.ts index 8d25a00..09afb2d 100644 --- a/src/commands/blob/sync.ts +++ b/src/commands/blob/sync.ts @@ -77,7 +77,7 @@ Examples: const filters = filtersOf(options); const action = transferAction(source, destination); const [sources, existing, destinationInside, sourceInside] = await Promise.all([ - listSource(source, true, resolver), + listSource(source, true, resolver, options.followSymlinks), listDestination(destination, resolver), nestedPrefix(source, destination, resolver), nestedPrefix(destination, source, resolver), @@ -123,7 +123,8 @@ Examples: const fileKey = async (path: string): Promise => localFileKey(await realLocalPath(path), true); const kept = new Set(await Promise.all(written.map(fileKey))); for (const [rel, entry] of current) { - if (wanted.has(rel)) continue; + // Deleting below a linked directory would delete outside the destination. + if (wanted.has(rel) || entry.linked) continue; if (entry.location.type === "local" && kept.has(await fileKey(entry.location.path))) continue; operations.push({ action: "delete", source: entry.location, size: 0 }); } diff --git a/src/commands/blob/transfer.ts b/src/commands/blob/transfer.ts index b1abb28..461fed3 100644 --- a/src/commands/blob/transfer.ts +++ b/src/commands/blob/transfer.ts @@ -166,6 +166,7 @@ export interface CommonOptions { } export interface ObjectOptions { + followSymlinks?: boolean; contentType?: string; cacheControl?: string; metadata?: Record; @@ -186,7 +187,8 @@ export function addObjectOptions(command: Command): Command { return command .option("--content-type ", "Content type for every written object (default: guessed from the file name, or kept on copies)") .option("--cache-control ", "Cache-Control for written objects: a header value, a duration like 1h, immutable, revalidate or no-store") - .option("--metadata ", "Metadata for written objects, as key=value[,key=value] or a JSON object", parseMetadata); + .option("--metadata ", "Metadata for written objects, as key=value[,key=value] or a JSON object", parseMetadata) + .option("--no-follow-symlinks", "Skip symbolic links in local source directories instead of following them"); } export function isDryRun(options: { dryrun?: boolean; dryRun?: boolean }): boolean { @@ -202,24 +204,31 @@ export interface Entry { /** Local mtime or the object's LastModified, in milliseconds. */ mtime: number; location: Location; + /** Reached through a symbolic link to a directory, so deleting it deletes a file outside the walked tree. */ + linked?: boolean; } function errorCode(error: unknown): string | undefined { return (error as NodeJS.ErrnoException | undefined)?.code; } -/** Symbolic links are followed as aws s3 does; one that loops back to its own ancestor is skipped. */ -async function walk(root: string): Promise { +/** + * Symbolic links are followed as aws s3 does, unless `follow` is false; one that loops back to its + * own ancestor is skipped. Files below a linked directory are marked, so nothing deletes them. + */ +async function walk(root: string, follow = true): Promise { const entries: Entry[] = []; - const visit = async (directory: string, ancestors: Set): Promise => { + const visit = async (directory: string, ancestors: Set, linked: boolean): Promise => { const real = await realpath(directory); if (ancestors.has(real)) return; const chain = new Set(ancestors).add(real); for await (const dirent of await opendir(directory)) { const path = join(directory, dirent.name); + const link = dirent.isSymbolicLink(); + if (link && !follow) continue; // A dangling link has nothing to copy. - const info = dirent.isSymbolicLink() ? await stat(path).catch(() => undefined) : dirent; - if (info?.isDirectory()) await visit(path, chain); + const info = link ? await stat(path).catch(() => undefined) : dirent; + if (info?.isDirectory()) await visit(path, chain, linked || link); else if (info?.isFile()) { const { size, mtimeMs } = await stat(path); entries.push({ @@ -227,11 +236,12 @@ async function walk(root: string): Promise { size, mtime: mtimeMs, location: { type: "local", path }, + ...(linked && { linked }), }); } } }; - await visit(root, new Set()); + await visit(root, new Set(), false); return entries; } @@ -260,7 +270,12 @@ export async function listBlobs(bucket: Bucket, location: BlobLocation, prefix: } /** What a cp, mv or sync reads: one file or object, or everything below a directory or prefix. */ -export async function listSource(location: Location, recursive: boolean, resolver: BucketResolver): Promise { +export async function listSource( + location: Location, + recursive: boolean, + resolver: BucketResolver, + followSymlinks = true, +): Promise { if (location.type === "local") { let info; try { @@ -271,7 +286,7 @@ export async function listSource(location: Location, recursive: boolean, resolve } if (info.isDirectory()) { if (!recursive) throw new Error(`${location.path} is a directory; use --recursive`); - return walk(location.path); + return walk(location.path, followSymlinks); } if (!info.isFile()) throw new Error(`${location.path} is not a regular file`); if (recursive) throw new Error(`${location.path} is a file; --recursive needs a directory`); diff --git a/tests/unit/blob-transfer.test.ts b/tests/unit/blob-transfer.test.ts index 75ef700..5397b4e 100644 --- a/tests/unit/blob-transfer.test.ts +++ b/tests/unit/blob-transfer.test.ts @@ -107,6 +107,43 @@ describe("sync --delete", () => { }); }); +describe("symbolic links to directories", () => { + beforeEach(async () => { + await mkdir(join(directory, "outside")); + await writeFile(join(directory, "outside", "precious.txt"), "keep"); + }); + + it("are never deleted through by sync --delete", async () => { + const dest = join(directory, "dest"); + await mkdir(dest); + await writeFile(join(dest, "stale.txt"), "old"); + await symlink(join(directory, "outside"), join(dest, "link")); + vi.spyOn(BucketResolver.prototype, "open").mockResolvedValue({ + list: async () => ({ blobs: [] }), + } as unknown as Bucket); + const result = await runCommand(await createBlobProgram(), ["blob", "sync", "blob://b/", dest, "-d", "-n", "-q"]); + expect(result).toEqual({ + dry_run: true, + operations: [{ action: "delete", source: join(dest, "stale.txt"), size: 0 }], + unchanged: 0, + }); + }); + + it("are refused as mv sources, and skipped with --no-follow-symlinks", async () => { + const source = join(directory, "src"); + await mkdir(source); + await writeFile(join(source, "a.txt"), "a"); + await symlink(join(directory, "outside"), join(source, "link")); + const mv = ["blob", "mv", source, "blob://b/dest", "-r", "-n", "-q"]; + await expect(runCommand(await createBlobProgram(), mv)).rejects.toThrow("1 entries would be skipped"); + const result = await runCommand(await createBlobProgram(), [...mv, "--no-follow-symlinks"]); + expect(result).toEqual({ + dry_run: true, + operations: [{ action: "move", source: join(source, "a.txt"), destination: "blob://b/dest/a.txt", size: 1 }], + }); + }); +}); + describe("putFile", () => { it("reopens the stream when retrying a transient failure", async () => { let attempts = 0;