README.md

@@ -268,9 +268,12 @@ are now tracked only in the [issues](https://git.eeqj.de/sneak/mfer/issues).
       cryptographic integrity of downloaded files. A file already there with the
       size and hash the manifest lists is skipped. Once every file is in place,
       the manifest is saved there as `index.mf`, so `mfer check` can verify the
-      tree later. A manifest that lists `index.mf` (in any letter case) or
+      tree later. Each file is downloaded to a temp file beside it, such as
-      `.index.mf.tmp` at the top of the tree is refused before any file is
+      `.a.txt.tmp` for `a.txt`, then moved into place. A manifest is refused
-      downloaded, since saving the manifest would replace it.
+      before any file is downloaded if it lists a file where fetch writes
+      another: at the temp file of a listed file, or at `index.mf` or
+      `.index.mf.tmp` at the top of the tree. Names are compared in any letter
+      case.
 - `mfer fetch --require-signature <fingerprint> https://example.com/stuff/`
     - as above, but first refuses a manifest not signed by the key with that
       fingerprint, as `mfer check --require-signature` does, before downloading

internal/cli/fetch.go

@@ -91,10 +91,10 @@ var (
 	// errHashMismatch indicates a downloaded file whose hash matches no
 	// manifest hash.
 	errHashMismatch = errors.New("hash mismatch")
-	// errManifestNameListed indicates a manifest that lists a file where
+	// errNameClash indicates a manifest that lists a file where fetch
-	// fetch saves the manifest.
+	// writes another file.
-	errManifestNameListed = errors.New(
+	errNameClash = errors.New(
-		"manifest lists a file where fetch saves the manifest")
+		"manifest lists a file where fetch writes another file")
 )
 
 // DownloadProgress reports the progress of a single file download.
@@ -412,7 +412,7 @@ func (mfa *CLIApp) fetchManifestOperation(ctx *cli.Context) error {
 
 // fetchManifest downloads the manifest at manifestURL and parses it,
 // enforcing --require-signature if it is given and refusing a manifest
-// that lists a file where it will be saved. It returns the manifest as
+// that lists a file where fetch writes another. It returns the manifest as
 // downloaded, to be saved once the files are in place, and the files it
 // lists.
 func fetchManifest(
@@ -452,7 +452,7 @@ func fetchManifest(
 
 	files := manifest.Files()
 
-	err = checkManifestNameUnlisted(files)
+	err = checkNoNameClash(files)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -462,19 +462,37 @@ func fetchManifest(
 	return manifestData, files, nil
 }
 
-// checkManifestNameUnlisted returns an error if files lists a file or
+// checkNoNameClash returns an error if files lists a file, or a directory
-// directory at the top of the tree under the name fetch saves the
+// a file is in, under a name where fetch writes another file: the temp
-// manifest as, or under that name's temp file. Saving the manifest would
+// file it downloads a listed file to, or, at the top of the tree, the
-// replace or remove it, or fail once every file was downloaded, leaving a
+// saved manifest or its temp file. fetch would remove or replace what is
-// tree check rejects. Names are compared ignoring case, since on a
+// listed there, or fail partway, leaving a tree check rejects. Names are
-// case-insensitive filesystem INDEX.MF and index.mf are one file.
+// compared ignoring case, since on a case-insensitive filesystem INDEX.MF
-func checkManifestNameUnlisted(files []*mfer.MFFilePath) error {
+// and index.mf are one file.
-	for _, f := range files {
+func checkNoNameClash(files []*mfer.MFFilePath) error {
-		top, _, _ := strings.Cut(filepath.Clean(f.GetPath()), string(filepath.Separator))
+	sep := string(filepath.Separator)
 
-		if strings.EqualFold(top, defaultManifestName) ||
+	// written maps each name fetch writes, other than the listed files
-			strings.EqualFold(top, tempPathFor(defaultManifestName)) {
+	// themselves, in lower case, to the file it writes there.
-			return fmt.Errorf("%w: %s", errManifestNameListed, f.GetPath())
+	written := map[string]string{
+		defaultManifestName:              "the saved manifest",
+		tempPathFor(defaultManifestName): "the saved manifest's temp file",
+	}
+
+	for _, f := range files {
+		tmpPath := tempPathFor(filepath.Clean(f.GetPath()))
+		written[strings.ToLower(tmpPath)] = "the temp file for " + f.GetPath()
+	}
+
+	for _, f := range files {
+		// Look up each directory on the file's path, then the file itself.
+		parts := strings.Split(strings.ToLower(filepath.Clean(f.GetPath())), sep)
+
+		for i := range parts {
+			what, ok := written[strings.Join(parts[:i+1], sep)]
+			if ok {
+				return fmt.Errorf("%w: %s (%s)", errNameClash, f.GetPath(), what)
+			}
 		}
 	}
 

internal/cli/fetch_test.go

@@ -1176,23 +1176,89 @@ func TestFetchRefusesListedManifestName(t *testing.T) {
 		t.Run(listed, func(t *testing.T) {
 			t.Parallel()
 
-			// Built directly rather than scanned, since a scan lists no
+			files := map[string][]byte{listed: []byte("listed")}
-			// hidden files and never a path starting with "./".
-			content := []byte("listed")
-			builder := mfer.NewBuilder()
-			_, err := builder.AddFile(mfer.RelFilePath(listed), mfer.FileSize(len(content)),
-				mfer.ModTime(time.Now()), bytes.NewReader(content), nil)
-			require.NoError(t, err)
 
-			var manifest bytes.Buffer
+			assertFetchRefused(t, builtManifest(t, files), files,
-			require.NoError(t, builder.Build(context.Background(), &manifest))
+				"manifest lists a file where fetch writes another file: "+listed)
-
-			assertFetchRefused(t, manifest.Bytes(), map[string][]byte{listed: content},
-				"manifest lists a file where fetch saves the manifest: "+listed)
 		})
 	}
 }
 
+// TestFetchRefusesListedTempName fetches manifests that list a.txt and
+// .a.txt.tmp, the temp file fetch downloads a.txt to, at the top of the
+// tree and in a directory. Downloading a.txt would remove .a.txt.tmp, so
+// fetch must refuse the manifest before it creates the destination or
+// requests any file. README with .README.tmp checks that temp names are
+// compared ignoring case. A manifest that lists only one of the two is
+// fetched in full.
+func TestFetchRefusesListedTempName(t *testing.T) {
+	t.Parallel()
+
+	for _, dir := range []string{"", "sub/"} {
+		for file, tmp := range map[string]string{
+			dir + "a.txt":  dir + ".a.txt.tmp",
+			dir + "README": dir + ".README.tmp",
+		} {
+			both := map[string][]byte{
+				file: []byte("a file"),
+				tmp:  []byte("a file at its temp name"),
+			}
+
+			t.Run(file+" and "+tmp, func(t *testing.T) {
+				t.Parallel()
+
+				assertFetchRefused(t, builtManifest(t, both), both,
+					"manifest lists a file where fetch writes another file: "+
+						tmp+" (the temp file for "+file+")")
+			})
+
+			for listed, content := range both {
+				t.Run("only "+listed, func(t *testing.T) {
+					t.Parallel()
+
+					files := map[string][]byte{listed: content}
+					manifest := builtManifest(t, files)
+
+					server := httptest.NewServer(fetchTestHandler(manifest, files))
+					defer server.Close()
+
+					dest := t.TempDir()
+
+					opts := testOpts([]string{
+						testApp, cmdFetch, "-q", "--" + flagDest, dest, server.URL,
+					}, afero.NewOsFs())
+					require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
+
+					want := maps.Clone(files)
+					want[defaultManifestName] = manifest
+					assert.Equal(t, want, filesUnder(t, dest))
+				})
+			}
+		}
+	}
+}
+
+// builtManifest returns a manifest of files, built directly rather than
+// scanned, since a scan lists no hidden files and never a path starting
+// with "./".
+func builtManifest(t *testing.T, files map[string][]byte) []byte {
+	t.Helper()
+
+	builder := mfer.NewBuilder()
+
+	for p, content := range files {
+		_, err := builder.AddFile(mfer.RelFilePath(p), mfer.FileSize(len(content)),
+			mfer.ModTime(time.Now()), bytes.NewReader(content), nil)
+		require.NoError(t, err)
+	}
+
+	var manifest bytes.Buffer
+
+	require.NoError(t, builder.Build(context.Background(), &manifest))
+
+	return manifest.Bytes()
+}
+
 // assertFetchRefused serves manifest, a manifest of files, and fetches it
 // with flags into a directory that does not exist yet. fetch must fail
 // with message after requesting only the manifest, and must not create