Never delete through symlinked directories, add --no-follow-symlinks

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

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.

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<string> => 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 });
         }

src/commands/blob/transfer.ts

@@ -166,6 +166,7 @@ export interface CommonOptions {
 }
 
 export interface ObjectOptions {
+  followSymlinks?: boolean;
   contentType?: string;
   cacheControl?: string;
   metadata?: Record<string, string>;
@@ -186,7 +187,8 @@ export function addObjectOptions(command: Command): Command {
   return command
     .option("--content-type <type>", "Content type for every written object (default: guessed from the file name, or kept on copies)")
     .option("--cache-control <value>", "Cache-Control for written objects: a header value, a duration like 1h, immutable, revalidate or no-store")
-    .option("--metadata <pairs>", "Metadata for written objects, as key=value[,key=value] or a JSON object", parseMetadata);
+    .option("--metadata <pairs>", "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,36 +204,44 @@ 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<Entry[]> {
+/**
+ * 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<Entry[]> {
   const entries: Entry[] = [];
-  const visit = async (directory: string, ancestors: Set<string>): Promise<void> => {
+  const visit = async (directory: string, ancestors: Set<string>, linked: boolean): Promise<void> => {
     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({
           rel: relative(root, path).split(sep).join("/"),
           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<Entry[]> {
+export async function listSource(
+  location: Location,
+  recursive: boolean,
+  resolver: BucketResolver,
+  followSymlinks = true,
+): Promise<Entry[]> {
   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`);

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;