fetch refuses a manifest that lists another file's temp name (closes #151)
check / check (push) Failing after 2s

fetch downloads each file to a temp name beside it and first removes
whatever is there. A manifest listing both a.txt and .a.txt.tmp had
fetch delete the second while fetching the first, then exit 0 with a
tree check rejects.

The refusal of a manifest that lists the saved manifest's own name or
temp name now covers this too: a listed file, or a directory a listed
file is in, may not sit at any name fetch writes besides the listed
files themselves. Temp names come from tempPathFor, names are compared
ignoring case as before, and the refusal still happens before the
destination is created or any file requested.

Model: opus-5-5
This commit was merged in pull request #153.
This commit is contained in:
2026-10-04 20:02:17 +02:00
parent d3394bd2a2
commit 9bb0ab3a03
3 changed files with 120 additions and 33 deletions
+36 -18
View File
@@ -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
// fetch saves the manifest.
errManifestNameListed = errors.New(
"manifest lists a file where fetch saves the manifest")
// errNameClash indicates a manifest that lists a file where fetch
// writes another file.
errNameClash = errors.New(
"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
// directory at the top of the tree under the name fetch saves the
// manifest as, or under that name's temp file. Saving the manifest would
// replace or remove it, or fail once every file was downloaded, leaving a
// tree check rejects. Names are compared ignoring case, since on a
// case-insensitive filesystem INDEX.MF and index.mf are one file.
func checkManifestNameUnlisted(files []*mfer.MFFilePath) error {
for _, f := range files {
top, _, _ := strings.Cut(filepath.Clean(f.GetPath()), string(filepath.Separator))
// 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
// 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.
func checkNoNameClash(files []*mfer.MFFilePath) error {
sep := string(filepath.Separator)
if strings.EqualFold(top, defaultManifestName) ||
strings.EqualFold(top, tempPathFor(defaultManifestName)) {
return fmt.Errorf("%w: %s", errManifestNameListed, f.GetPath())
// written maps each name fetch writes, other than the listed files
// themselves, in lower case, to the file it writes there.
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)
}
}
}