From 46f856df90c7bc2d0f6af1349710b45fc40226fb Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Mon, 5 Oct 2026 22:57:16 +0000 Subject: [PATCH] fetch refuses a manifest whose paths differ only in letter case (closes #154) On a case-insensitive filesystem such paths are one name. Of two files, one replaced the other and fetch exited 0. For a file and a directory another file is in, fetch stopped partway with a non-zero exit, leaving a partial tree. For two spellings of one directory, both files landed in one directory, one under a spelling the manifest does not list, and check reported that file as not in the manifest. The existing name-clash check now also records each listed file and each directory one is in. A listed file at a name already taken, or a directory spelled differently from one already there, is refused on every filesystem, before the destination is created. The message names both paths. Model: opus-5-5 --- README.md | 8 +++-- internal/cli/fetch.go | 37 ++++++++++++++-------- internal/cli/fetch_test.go | 63 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 98d5446..8f36782 100644 --- a/README.md +++ b/README.md @@ -274,9 +274,11 @@ are now tracked only in the [issues](https://git.eeqj.de/sneak/mfer/issues). tree later. Each file is downloaded to a temp file beside it, such as `.a.txt.tmp` for `a.txt`, then moved into place. A manifest is refused 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. + another: at another listed file or a directory one is in, 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, on every filesystem, since on + a case-insensitive one `A.txt` and `a.txt` are one file; a directory two + listed files are in must be spelled alike in both. - `mfer fetch --require-signature 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 diff --git a/internal/cli/fetch.go b/internal/cli/fetch.go index 966a771..cc0a8aa 100644 --- a/internal/cli/fetch.go +++ b/internal/cli/fetch.go @@ -466,17 +466,18 @@ func fetchManifest( } // checkNoNameClash returns an error if files lists a file, or a directory -// a file is in, under a name where fetch writes another file: the temp -// file it downloads a listed file to, or, at the top of the tree, the -// saved manifest or its temp file. fetch would remove or replace what is +// a file is in, under a name where fetch writes another file: another +// listed file, a directory another listed file is in, the temp file it +// downloads a listed file to, or, at the top of the tree, the saved +// manifest or its temp file. fetch would remove or replace what is // listed there, or fail partway, leaving a tree check rejects. Names are -// compared ignoring case, since on a case-insensitive filesystem INDEX.MF -// and index.mf are one file. +// compared ignoring case, on every filesystem, since on a +// case-insensitive one A.txt and a.txt are one file. func checkNoNameClash(files []*mfer.MFFilePath) error { sep := string(filepath.Separator) - // written maps each name fetch writes, other than the listed files - // themselves, in lower case, to the file it writes there. + // written maps each name fetch writes, in lower case, to the file or + // directory it writes there. written := map[string]string{ defaultManifestName: "the saved manifest", tempPathFor(defaultManifestName): "the saved manifest's temp file", @@ -488,14 +489,26 @@ func checkNoNameClash(files []*mfer.MFFilePath) error { } 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) + // Look up and add each directory on the file's path, then the + // file itself. Only the same directory, spelled alike, may + // already be there. + parts := strings.Split(filepath.Clean(f.GetPath()), sep) + last := len(parts) - 1 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) + name := strings.Join(parts[:i+1], sep) + + what := "the directory " + name + if i == last { + what = "the file " + f.GetPath() } + + other, ok := written[strings.ToLower(name)] + if ok && (i == last || other != what) { + return fmt.Errorf("%w: %s (%s)", errNameClash, f.GetPath(), other) + } + + written[strings.ToLower(name)] = what } } diff --git a/internal/cli/fetch_test.go b/internal/cli/fetch_test.go index 10e263c..da9a434 100644 --- a/internal/cli/fetch_test.go +++ b/internal/cli/fetch_test.go @@ -1236,6 +1236,69 @@ func TestFetchRefusesListedTempName(t *testing.T) { } } +// TestFetchRefusesNamesEqualIgnoringCase fetches manifests that list two +// paths that are one name on a case-insensitive filesystem. Fetched +// there, of two such files, at the top of the tree or in a directory, +// one replaces the other and fetch exits 0. A file and a directory +// another file is in stop fetch partway with a non-zero exit, leaving a +// partial tree. Two spellings of one directory put both files in one +// directory, one of them under a spelling the manifest does not list, +// and check reports that file as not in the manifest. So fetch must +// refuse each on every filesystem before it creates the destination or +// requests any file. A file and a directory with the same name, and a +// file listed twice, are refused the same way. A manifest whose names +// differ in more than letter case is fetched in full. +func TestFetchRefusesNamesEqualIgnoringCase(t *testing.T) { + t.Parallel() + + for _, tc := range []struct{ first, second, message string }{ + {"X.txt", "x.txt", "x.txt (the file X.txt)"}, + {"sub/X.txt", "sub/x.txt", "sub/x.txt (the file sub/X.txt)"}, + {"Dir", "dir/x", "dir/x (the file Dir)"}, + {"Dir/a.txt", "dir/b.txt", "dir/b.txt (the directory Dir)"}, + {"dir", "dir/x", "dir/x (the file dir)"}, + {"./x.txt", "x.txt", "x.txt (the file ./x.txt)"}, + } { + t.Run(tc.first+" and "+tc.second, func(t *testing.T) { + t.Parallel() + + files := map[string][]byte{ + tc.first: []byte("the first file"), + tc.second: []byte("the second file"), + } + + assertFetchRefused(t, builtManifest(t, files), files, + "manifest lists a file where fetch writes another file: "+tc.message) + }) + } + + t.Run("names that differ in more than letter case", func(t *testing.T) { + t.Parallel() + + files := map[string][]byte{ + "A.txt": []byte("at the top"), + "B.txt": []byte("also at the top"), + "dir/a.txt": []byte("in a directory"), + "dir/b.txt": []byte("in the same directory"), + } + 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 "./". -- 2.54.0