fetch: destination directory, skip files already present, save the manifest, require a signer (closes #101)
check / check (push) Failing after 3s

fetch takes --dest (default .) and writes every file there through the
existing symlink and hard-link guards, which now work relative to that
directory. A file already there with the listed size and hash is
skipped; a leftover temp file is still replaced. Once every file
verifies, the manifest is saved as index.mf through the same temp file
and rename, so check runs on the result; a manifest that lists index.mf
or its temp name at the top of the tree is refused, since saving would
replace that file. --require-signature is shared with check and
enforced through verifyRequiredSigner. Both refusals come before any
file is downloaded or anything is written.

Model: opus-5-5
This commit is contained in:
2026-10-04 16:05:22 +00:00
parent acff23d92b
commit df985e44dc
6 changed files with 641 additions and 153 deletions
+17 -6
View File
@@ -43,9 +43,11 @@ bin/mfer gen .
# it lists is missing or corrupted; warns about files it does not list.
bin/mfer check index.mf
# Download and cryptographically verify a tree published over HTTP: mfer
# fetches <url>/index.mf, then downloads every file it lists.
bin/mfer fetch https://example.com/tree/
# Download and cryptographically verify a tree published over HTTP into
# ./mirror: mfer fetches <url>/index.mf, downloads every file it lists,
# skipping any already there with the right hash, then saves the manifest as
# mirror/index.mf.
bin/mfer fetch --dest mirror https://example.com/tree/
```
Run `bin/mfer help` for the full command list, or `bin/mfer <command> --help`
@@ -258,9 +260,18 @@ are now tracked only in the [issues](https://git.eeqj.de/sneak/mfer/issues).
list, hidden files included; with `--no-extra-files` each one is a failure
instead
- `mfer fetch https://example.com/stuff/`
- fetches `/stuff/index.mf` and downloads all files listed in manifest,
optionally resuming any that already exist locally, and assures
cryptographic integrity of downloaded files.
- fetches `/stuff/index.mf` and downloads all files listed in manifest into
the current directory, or the one given with `--dest`, and assures
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
`.index.mf.tmp` at the top of the tree is refused before any file is
downloaded, since saving the manifest would replace it.
- `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
any file.
# Implementation Plan
+1 -1
View File
@@ -319,7 +319,7 @@ func (mfa *CLIApp) checkManifestOperation(ctx *cli.Context) error {
}
// Check signature requirement
requiredSigner := ctx.String("require-signature")
requiredSigner := ctx.String(flagRequireSignature)
if requiredSigner != "" {
err = verifyRequiredSigner(ctx.Context, chk, requiredSigner)
if err != nil {
+22 -12
View File
@@ -111,9 +111,10 @@ func TestVerifyRequiredSignerMessages(t *testing.T) {
// string; the required signer is a fixed value that cannot match it. Requires
// gpg and is skipped where it is absent, as the other signing tests are.
//
//nolint:paralleltest // signedChecker calls t.Setenv, which bars t.Parallel
//nolint:paralleltest // signedManifest calls t.Setenv, which bars t.Parallel
func TestSignerMismatchMessage(t *testing.T) {
chk := signedChecker(t)
chk := signedChecker(t,
signedManifest(t, map[string][]byte{"f.txt": []byte("signed file")}))
embeddedFP, err := chk.ExtractEmbeddedSigningKeyFP(context.Background())
require.NoError(t, err)
@@ -125,9 +126,10 @@ func TestSignerMismatchMessage(t *testing.T) {
" does not match required "+msgFpB)
}
// signedChecker builds a Checker over a manifest signed by a throwaway GPG
// key generated in a temporary GNUPGHOME.
func signedChecker(t *testing.T) *mfer.Checker {
// signedManifest returns a manifest of files signed by a throwaway GPG key
// generated in a temporary GNUPGHOME, which it leaves set for the rest of
// the test.
func signedManifest(t *testing.T, files map[string][]byte) []byte {
t.Helper()
_, err := exec.LookPath("gpg")
@@ -159,17 +161,25 @@ func signedChecker(t *testing.T) *mfer.Checker {
b := mfer.NewBuilder()
b.SetSigningOptions(&mfer.SigningOptions{KeyID: mfer.GPGKeyID("test@mfer.test")})
content := []byte("signed file")
_, err = b.AddFile("f.txt", mfer.FileSize(len(content)), mfer.ModTime{},
bytes.NewReader(content), nil)
require.NoError(t, err)
for path, content := range files {
_, err = b.AddFile(mfer.RelFilePath(path), mfer.FileSize(len(content)),
mfer.ModTime{}, bytes.NewReader(content), nil)
require.NoError(t, err)
}
var buf bytes.Buffer
require.NoError(t, b.Build(context.Background(), &buf))
return buf.Bytes()
}
// signedChecker builds a Checker over manifest, a signed manifest.
func signedChecker(t *testing.T, manifest []byte) *mfer.Checker {
t.Helper()
fs := afero.NewMemMapFs()
require.NoError(t, afero.WriteFile(fs, "/index.mf", buf.Bytes(), 0o644))
require.NoError(t, afero.WriteFile(fs, "/index.mf", manifest, 0o644))
chk, err := mfer.NewChecker(&mfer.CheckerOptions{
ManifestPath: "/index.mf",
@@ -300,7 +310,7 @@ func TestFetchFileHTTPStatusMessage(t *testing.T) {
// downloadFile logs each retry of the 500 to the process-global logger.
err := runLocked(func() error {
return downloadFile(context.Background(), testClient(), server.URL+"/x", "x",
return downloadFile(context.Background(), testClient(), server.URL+"/x", ".", "x",
&mfer.MFFilePath{}, nil)
})
require.ErrorIs(t, err, errHTTPStatus)
@@ -355,7 +365,7 @@ func TestSizeMismatchMessage(t *testing.T) {
// finishDownload returns the size-mismatch error before it touches the
// paths, digest, or entry, so those can be zero here.
err := finishDownload("", "", 9, 10, nil, nil, nil, nil)
err := finishDownload("", "", "", 9, 10, nil, nil, nil, nil)
require.ErrorIs(t, err, errSizeMismatch)
assert.EqualError(t, err, "size mismatch: expected 10 bytes, got 9")
}
+286 -93
View File
@@ -19,6 +19,7 @@ import (
"github.com/dustin/go-humanize"
"github.com/multiformats/go-multihash"
"github.com/spf13/afero"
"github.com/urfave/cli/v2"
"sneak.berlin/go/mfer/internal/log"
"sneak.berlin/go/mfer/mfer"
@@ -90,6 +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")
)
// DownloadProgress reports the progress of a single file download.
@@ -239,20 +244,34 @@ func manifestBaseURL(manifestURL string) (*url.URL, error) {
return parsed.JoinPath(".."), nil
}
// downloadManifestFiles downloads every file in the manifest, reporting
// progress on the progress channel.
// downloadManifestFiles downloads every file in the manifest into dest,
// reporting progress on the progress channel. A file already present in
// dest is skipped. It returns how many files it downloaded and their
// total size.
func downloadManifestFiles(
ctx context.Context,
client retryingClient,
baseURL *url.URL,
dest string,
files []*mfer.MFFilePath,
progress chan<- DownloadProgress,
) error {
) (int, int64, error) {
var (
downloaded int
downloadedBytes int64
)
for _, f := range files {
// Sanitize the path to prevent path traversal attacks
localPath, err := sanitizePath(f.GetPath())
if err != nil {
return fmt.Errorf("invalid path in manifest: %w", err)
return 0, 0, fmt.Errorf("invalid path in manifest: %w", err)
}
if alreadyPresent(dest, localPath, f) {
log.Infof("skipping %s: already present", f.GetPath())
continue
}
// JoinPath takes escaped path text, so a name such as "100%.txt"
@@ -260,13 +279,53 @@ func downloadManifestFiles(
fileURL := baseURL.JoinPath(encodeFilePath(f.GetPath())).String()
log.Infof("fetching %s", f.GetPath())
err = downloadFile(ctx, client, fileURL, localPath, f, progress)
err = downloadFile(ctx, client, fileURL, dest, localPath, f, progress)
if err != nil {
return fmt.Errorf("failed to download %s: %w", f.GetPath(), err)
return 0, 0, fmt.Errorf("failed to download %s: %w", f.GetPath(), err)
}
downloaded++
downloadedBytes += f.GetSize()
}
return nil
return downloaded, downloadedBytes, nil
}
// alreadyPresent reports whether localPath under dest is a regular file
// with the size and one of the hashes the manifest lists for entry. It
// hashes the whole file, since a matching size alone would accept a
// corrupted or partly written one. A file it cannot read, or reaches only
// through a symlink, is not present: fetch downloads it, and the download
// reports the problem.
func alreadyPresent(dest, localPath string, entry *mfer.MFFilePath) bool {
if checkNoSymlinks(dest, localPath) != nil {
return false
}
path := filepath.Join(dest, localPath)
info, err := os.Lstat(path)
if err != nil || !info.Mode().IsRegular() || info.Size() != entry.GetSize() {
return false
}
// G304: localPath is a relative path that sanitizePath keeps inside
// dest as text, and checkNoSymlinks just found no symlink in it.
f, err := os.Open(path) //nolint:gosec // G304: see comment above
if err != nil {
return false
}
defer func() { _ = f.Close() }()
h := sha256.New()
_, err = io.Copy(h, f)
if err != nil {
return false
}
return verifyDownloadedHash(h.Sum(nil), entry) == nil
}
func (mfa *CLIApp) fetchManifestOperation(ctx *cli.Context) error {
@@ -291,43 +350,22 @@ func (mfa *CLIApp) fetchManifestOperation(ctx *cli.Context) error {
firstDelay: firstRetryDelay,
}
log.Infof("fetching manifest from %s", manifestURL)
// Read the whole manifest before parsing it, so that a connection
// lost partway through is retried rather than reported as a bad
// manifest.
var manifestData []byte
err = client.get(ctx.Context, manifestURL, func(resp *http.Response) error {
var readErr error
manifestData, readErr = io.ReadAll(resp.Body)
return readErr
})
manifestData, files, err := fetchManifest(ctx, client, manifestURL)
if err != nil {
return fmt.Errorf("failed to fetch manifest: %w", err)
return err
}
// Parse manifest
manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestData))
if err != nil {
return fmt.Errorf("failed to parse manifest: %w", err)
}
files := manifest.Files()
log.Infof("manifest contains %d files", len(files))
// Compute base URL (directory containing manifest)
baseURL, err := manifestBaseURL(manifestURL)
if err != nil {
return err
}
// Calculate total bytes to download
var totalBytes int64
for _, f := range files {
totalBytes += f.GetSize()
dest := ctx.String(flagDest)
err = os.MkdirAll(dest, dirPerms)
if err != nil {
return fmt.Errorf("failed to create destination directory %s: %w", dest, err)
}
// Create progress channel and start progress reporter goroutine
@@ -340,7 +378,8 @@ func (mfa *CLIApp) fetchManifestOperation(ctx *cli.Context) error {
startTime := time.Now()
// Download each file
dlErr := downloadManifestFiles(ctx.Context, client, baseURL, files, progress)
downloaded, downloadedBytes, dlErr := downloadManifestFiles(
ctx.Context, client, baseURL, dest, files, progress)
close(progress)
<-done
@@ -349,15 +388,150 @@ func (mfa *CLIApp) fetchManifestOperation(ctx *cli.Context) error {
return dlErr
}
// Saved only now that every file is in place and verified, so that
// "mfer check" can verify the tree later.
err = saveManifest(dest, manifestData)
if err != nil {
return fmt.Errorf("failed to save manifest: %w", err)
}
// Print summary
elapsed := time.Since(startTime)
avgBytesPerSec := float64(totalBytes) / elapsed.Seconds()
avgBytesPerSec := float64(downloadedBytes) / elapsed.Seconds()
avgRate := formatBitrate(avgBytesPerSec * bitsPerByte)
log.Infof("downloaded %d files (%s) in %.1fs (%s avg)",
len(files),
humanize.IBytes(safeUint64(totalBytes)),
log.Infof("downloaded %d files (%s) in %.1fs (%s avg), skipped %d already present",
downloaded,
humanize.IBytes(safeUint64(downloadedBytes)),
elapsed.Seconds(),
avgRate)
avgRate,
len(files)-downloaded)
log.Infof("saved manifest to %s", filepath.Join(dest, defaultManifestName))
return nil
}
// 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
// downloaded, to be saved once the files are in place, and the files it
// lists.
func fetchManifest(
ctx *cli.Context, client retryingClient, manifestURL string,
) ([]byte, []*mfer.MFFilePath, error) {
log.Infof("fetching manifest from %s", manifestURL)
// Read the whole manifest before parsing it, so that a connection
// lost partway through is retried rather than reported as a bad
// manifest.
var manifestData []byte
err := client.get(ctx.Context, manifestURL, func(resp *http.Response) error {
var readErr error
manifestData, readErr = io.ReadAll(resp.Body)
return readErr
})
if err != nil {
return nil, nil, fmt.Errorf("failed to fetch manifest: %w", err)
}
// Parse manifest
manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestData))
if err != nil {
return nil, nil, fmt.Errorf("failed to parse manifest: %w", err)
}
requiredSigner := ctx.String(flagRequireSignature)
if requiredSigner != "" {
err = verifyFetchedSigner(ctx, manifestData, requiredSigner)
if err != nil {
return nil, nil, err
}
}
files := manifest.Files()
err = checkManifestNameUnlisted(files)
if err != nil {
return nil, nil, err
}
log.Infof("manifest contains %d files", len(files))
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))
if strings.EqualFold(top, defaultManifestName) ||
strings.EqualFold(top, tempPathFor(defaultManifestName)) {
return fmt.Errorf("%w: %s", errManifestNameListed, f.GetPath())
}
}
return nil
}
// verifyFetchedSigner enforces --require-signature on the fetched manifest
// exactly as check does. verifyRequiredSigner takes a Checker, which loads
// its manifest from a file, so the manifest is handed to it as a file in
// memory.
func verifyFetchedSigner(
ctx *cli.Context, manifestData []byte, requiredSigner string,
) error {
memFs := afero.NewMemMapFs()
manifestPath := "/" + defaultManifestName
err := afero.WriteFile(memFs, manifestPath, manifestData, filePerms)
if err != nil {
return err
}
chk, err := mfer.NewChecker(&mfer.CheckerOptions{
ManifestPath: manifestPath,
BasePath: "/",
Fs: memFs,
})
if err != nil {
return fmt.Errorf("failed to load manifest: %w", err)
}
return verifyRequiredSigner(ctx.Context, chk, requiredSigner)
}
// saveManifest writes the fetched manifest into dest under the default
// manifest name, the way fetch writes every file: to a new temp file that
// is then renamed into place.
func saveManifest(dest string, manifestData []byte) error {
tmpPath := tempPathFor(defaultManifestName)
out, err := createTempFile(dest, tmpPath)
if err != nil {
return err
}
_, writeErr := out.Write(manifestData)
closeErr := out.Close()
err = errors.Join(writeErr, closeErr)
if err == nil {
err = moveIntoPlace(dest, tmpPath, defaultManifestName)
}
if err != nil {
_ = os.Remove(filepath.Join(dest, tmpPath))
return err
}
return nil
}
@@ -401,14 +575,15 @@ func sanitizePath(p string) (string, error) {
return cleaned, nil
}
// checkNoSymlinks returns an error if any part of the relative path p
// already exists as a symlink. sanitizePath checks p only as text, so
// without this a symlink inside the target directory could send a write
// to p outside of it. Parts that do not exist yet are fine: fetch creates
// them as plain directories and files. Call it immediately before each
// write: a symlink created after it returns is not caught.
func checkNoSymlinks(p string) error {
current := ""
// checkNoSymlinks returns an error if any part of p, a path relative to
// dest, already exists under dest as a symlink. dest itself is the user's
// choice and may be one. sanitizePath checks p only as text, so without
// this a symlink inside dest could send a write to p outside of it. Parts
// that do not exist yet are fine: fetch creates them as plain directories
// and files. Call it immediately before each write: a symlink created
// after it returns is not caught.
func checkNoSymlinks(dest, p string) error {
current := dest
for _, part := range strings.Split(p, string(filepath.Separator)) {
current = filepath.Join(current, part)
@@ -548,13 +723,14 @@ func verifyDownloadedHash(digest []byte, entry *mfer.MFFilePath) error {
return errHashMismatch
}
// downloadFile downloads a URL to a local file path with hash verification.
// It downloads to a temporary file, verifies the hash, then renames to the final path.
// Progress is reported via the progress channel.
// downloadFile downloads a URL to localPath, a path relative to dest, with
// hash verification. It downloads to a temporary file, verifies the hash,
// then renames to the final path. Progress is reported via the progress
// channel.
func downloadFile(
ctx context.Context,
client retryingClient,
fileURL, localPath string,
fileURL, dest, localPath string,
entry *mfer.MFFilePath,
progress chan<- DownloadProgress,
) error {
@@ -568,11 +744,13 @@ func downloadFile(
// Create parent directories if needed
dir := filepath.Dir(localPath)
if dir != "" && dir != "." {
err = checkNoSymlinks(dir)
err = checkNoSymlinks(dest, dir)
if err != nil {
return err
}
dir = filepath.Join(dest, dir)
err = os.MkdirAll(dir, dirPerms)
if err != nil {
return fmt.Errorf("failed to create directory %s: %w", dir, err)
@@ -582,17 +760,61 @@ func downloadFile(
tmpPath := tempPathFor(localPath)
return client.get(ctx, fileURL, func(resp *http.Response) error {
return saveResponse(resp, tmpPath, localPath, entry, progress)
return saveResponse(resp, dest, tmpPath, localPath, entry, progress)
})
}
// createTempFile creates tmpPath, a path relative to dest, as a new empty
// file.
func createTempFile(dest, tmpPath string) (*os.File, error) {
err := checkNoSymlinks(dest, tmpPath)
if err != nil {
return nil, err
}
path := filepath.Join(dest, tmpPath)
// Remove whatever is at tmpPath, such as a leftover from an
// interrupted run, rather than write into it: it may be a hard link
// to a file outside dest, and removing a hard link removes only this
// name. If the removal fails, O_EXCL below makes the create fail.
_ = os.Remove(path)
// Create the temp file only if nothing is at tmpPath (O_EXCL).
//
// G304: tmpPath is a relative path that sanitizePath keeps inside dest
// as text, and checkNoSymlinks just found no symlink in it.
out, err := os.OpenFile( //nolint:gosec // G304: see comment above
path, os.O_RDWR|os.O_CREATE|os.O_EXCL, filePerms)
if err != nil {
return nil, fmt.Errorf("failed to create temp file: %w", err)
}
return out, nil
}
// moveIntoPlace renames tmpPath to localPath, both relative to dest.
func moveIntoPlace(dest, tmpPath, localPath string) error {
err := checkNoSymlinks(dest, localPath)
if err != nil {
return err
}
err = os.Rename(filepath.Join(dest, tmpPath), filepath.Join(dest, localPath))
if err != nil {
return fmt.Errorf("failed to rename temp file: %w", err)
}
return nil
}
// saveResponse writes resp's body to tmpPath, verifies it against entry,
// and renames it to localPath. It starts a new temp file each time and
// removes it on failure, so a retry after a failed try never appends to
// or keeps a partial file.
// and renames it to localPath, both paths relative to dest. It starts a
// new temp file each time and removes it on failure, so a retry after a
// failed try never appends to or keeps a partial file.
func saveResponse(
resp *http.Response,
tmpPath, localPath string,
dest, tmpPath, localPath string,
entry *mfer.MFFilePath,
progress chan<- DownloadProgress,
) error {
@@ -604,29 +826,11 @@ func saveResponse(
totalBytes = expectedSize
}
err := checkNoSymlinks(tmpPath)
out, err := createTempFile(dest, tmpPath)
if err != nil {
return err
}
// Remove whatever is at tmpPath, such as a leftover from an
// interrupted run, rather than write into it: it may be a hard link
// to a file outside the target directory, and removing a hard link
// removes only this name. If the removal fails, O_EXCL below makes
// the create fail.
_ = os.Remove(tmpPath)
// Create the temp file only if nothing is at tmpPath (O_EXCL).
//
// G304: tmpPath is a relative path that sanitizePath keeps inside the
// target directory as text, and checkNoSymlinks just found no symlink
// in it.
out, err := os.OpenFile( //nolint:gosec // G304: see comment above
tmpPath, os.O_RDWR|os.O_CREATE|os.O_EXCL, filePerms)
if err != nil {
return fmt.Errorf("failed to create temp file: %w", err)
}
// Set up hash computation
h := sha256.New()
@@ -646,10 +850,10 @@ func saveResponse(
closeErr := out.Close()
err = finishDownload(
tmpPath, localPath, written, expectedSize, h.Sum(nil), entry,
dest, tmpPath, localPath, written, expectedSize, h.Sum(nil), entry,
copyErr, closeErr)
if err != nil {
_ = os.Remove(tmpPath)
_ = os.Remove(filepath.Join(dest, tmpPath))
return err
}
@@ -660,7 +864,7 @@ func saveResponse(
// finishDownload validates the copy result, verifies size and hash, and
// moves the temp file into place. On error the caller removes tmpPath.
func finishDownload(
tmpPath, localPath string,
dest, tmpPath, localPath string,
written, expectedSize int64,
digest []byte,
entry *mfer.MFFilePath,
@@ -686,16 +890,5 @@ func finishDownload(
return err
}
err = checkNoSymlinks(localPath)
if err != nil {
return err
}
// Rename temp file to final path
err = os.Rename(tmpPath, localPath)
if err != nil {
return fmt.Errorf("failed to rename temp file: %w", err)
}
return nil
return moveIntoPlace(dest, tmpPath, localPath)
}
+292 -33
View File
@@ -13,6 +13,7 @@ import (
"net/http/httptest"
"os"
"path/filepath"
"slices"
"strconv"
"sync"
"sync/atomic"
@@ -338,7 +339,7 @@ func TestFetchFromHTTP(t *testing.T) {
fileURL := baseURL + f.GetPath()
err = downloadFile(context.Background(), testClient(),
fileURL, localPath, f, progress)
fileURL, ".", localPath, f, progress)
require.NoError(t, err, "failed to download %s", f.GetPath())
}
@@ -386,7 +387,7 @@ func TestFetchHashMismatch(t *testing.T) {
// Try to download - should fail with hash mismatch
err = downloadFile(context.Background(), testClient(),
server.URL+"/file.txt", testFileTxt, files[0], nil)
server.URL+"/file.txt", ".", testFileTxt, files[0], nil)
require.Error(t, err)
assert.Contains(t, err.Error(), "mismatch")
@@ -432,7 +433,7 @@ func TestFetchSizeMismatch(t *testing.T) {
// Try to download - should fail with size mismatch
err = downloadFile(context.Background(), testClient(),
server.URL+"/file.txt", testFileTxt, files[0], nil)
server.URL+"/file.txt", ".", testFileTxt, files[0], nil)
require.Error(t, err)
assert.Contains(t, err.Error(), "size mismatch")
@@ -490,7 +491,7 @@ func TestFetchProgress(t *testing.T) {
// Download
err = downloadFile(context.Background(), testClient(),
server.URL+"/large.txt", "large.txt", files[0], progress)
server.URL+"/large.txt", ".", "large.txt", files[0], progress)
close(progress)
<-done
@@ -513,24 +514,51 @@ func TestFetchProgress(t *testing.T) {
assert.Equal(t, content, downloaded)
}
// TestFetchRefusesSymlinks runs fetch into a destination directory that
// holds a symlink pointing outside it, in each of the three places fetch
// writes: a parent directory, the temp file, and the file itself, which
// the temp file is renamed onto; and once as a directory inside a plain
// directory. The fetch must fail and nothing outside may change.
// TestFetchRefusesSymlinks runs fetch with --dest naming a directory other
// than the current one, which holds a symlink pointing outside it, in each
// place fetch writes: a parent directory, the temp file, the file itself,
// which the temp file is renamed onto, and the saved manifest's temp file
// and final name; and once as a directory inside a plain directory. The
// fetch must fail, and neither the outside directory nor the current one
// may change.
//
//nolint:paralleltest // changes the process-global working directory
func TestFetchRefusesSymlinks(t *testing.T) {
// What a link standing for a file points to: a file outside that does
// not exist yet.
const newFile = "new.txt"
tests := []struct {
name string
entry string // the manifest's only file
link string // symlink placed in the destination directory
target string // what link points to, relative to the outside directory
name string
entry string // the manifest's only file
link string // symlink placed in the destination directory
target string // what link points to, relative to the outside directory
failure string // what fetch reports it was doing when it found link
}{
{"parent directory", "sub/deeper/file.txt", "sub", "."},
{"directory inside a plain directory", "docs/data/passwd", "docs/data", "."},
{"temp file", testFileTxt, ".file.txt.tmp", "new.txt"},
{"file", testFileTxt, testFileTxt, "new.txt"},
{
"parent directory", "sub/deeper/file.txt", "sub", ".",
"failed to download sub/deeper/file.txt",
},
{
"directory inside a plain directory", "docs/data/passwd", "docs/data", ".",
"failed to download docs/data/passwd",
},
{
"temp file", testFileTxt, ".file.txt.tmp", newFile,
"failed to download " + testFileTxt,
},
{
"file", testFileTxt, testFileTxt, newFile,
"failed to download " + testFileTxt,
},
{
"manifest temp file", testFileTxt, tempPathFor(defaultManifestName), newFile,
"failed to save manifest",
},
{
"manifest", testFileTxt, defaultManifestName, newFile,
"failed to save manifest",
},
}
for _, tt := range tests {
@@ -545,23 +573,61 @@ func TestFetchRefusesSymlinks(t *testing.T) {
defer server.Close()
outside := t.TempDir()
cwd := chdirTemp(t)
dest := t.TempDir()
link := filepath.Join(dest, tt.link)
chdirTemp(t)
require.NoError(t, os.MkdirAll(filepath.Dir(tt.link), 0o750))
require.NoError(t, os.Symlink(filepath.Join(outside, tt.target), tt.link))
require.NoError(t, os.MkdirAll(filepath.Dir(link), 0o750))
require.NoError(t, os.Symlink(filepath.Join(outside, tt.target), link))
opts := testOpts([]string{testApp, cmdFetch, "-q", server.URL}, afero.NewOsFs())
opts := testOpts([]string{
testApp, cmdFetch, "-q", "--" + flagDest, dest, server.URL,
}, afero.NewOsFs())
assert.Equal(t, 1, runCLI(opts))
assert.Contains(t, testStderr(t, opts), "failed to download "+tt.entry+
": symlink in path not allowed: "+tt.link)
assert.Contains(t, testStderr(t, opts),
tt.failure+": symlink in path not allowed: "+link)
written, err := os.ReadDir(outside)
require.NoError(t, err)
assert.Empty(t, written, "fetch wrote outside the destination")
written, err = os.ReadDir(cwd)
require.NoError(t, err)
assert.Empty(t, written, "fetch wrote to the current directory")
})
}
}
// TestFetchDoesNotSkipThroughSymlink runs fetch with --dest holding a
// symlink to a directory outside it, where the file the manifest lists
// through that symlink already sits with the listed content. fetch must
// not take that file as already present: it must fail on the symlink, as
// the download would, and leave the outside file alone.
func TestFetchDoesNotSkipThroughSymlink(t *testing.T) {
t.Parallel()
content := []byte("fetched")
files := map[string][]byte{"sub/" + testFileTxt: content}
server := httptest.NewServer(fetchTestHandler(manifestOf(t, files), files))
defer server.Close()
outside := t.TempDir()
require.NoError(t, os.WriteFile(filepath.Join(outside, testFileTxt), content, 0o600))
dest := t.TempDir()
link := filepath.Join(dest, "sub")
require.NoError(t, os.Symlink(outside, link))
opts := testOpts([]string{
testApp, cmdFetch, "-q", "--" + flagDest, dest, server.URL,
}, afero.NewOsFs())
assert.Equal(t, 1, runCLI(opts))
assert.Contains(t, testStderr(t, opts),
"failed to download sub/"+testFileTxt+": symlink in path not allowed: "+link)
assert.Equal(t, map[string][]byte{testFileTxt: content}, filesUnder(t, outside))
}
// TestFetchReplacesHardLinkAtTempName runs fetch into a destination
// directory that holds, at the temp file's name, a hard link to a file
// outside it. To fetch that is an ordinary leftover from an interrupted
@@ -787,7 +853,7 @@ func TestDownloadFileRetriesToSuccess(t *testing.T) {
chdirTemp(t)
err = downloadFile(context.Background(), testClient(),
server.URL+"/"+testFileTxt, testFileTxt, manifest.Files()[0], nil)
server.URL+"/"+testFileTxt, ".", testFileTxt, manifest.Files()[0], nil)
require.NoError(t, err)
assert.Equal(t, int32(3), requests.Load())
@@ -878,7 +944,7 @@ func TestFetchEscapedPaths(t *testing.T) {
// TestFetchTree runs fetch on a tree with nested directories. Every file
// the manifest lists must land under its own path with its own content,
// and nothing else may be left in the destination.
// beside the manifest, and nothing else may be left in the destination.
//
//nolint:paralleltest // changes the process-global working directory
func TestFetchTree(t *testing.T) {
@@ -889,7 +955,9 @@ func TestFetchTree(t *testing.T) {
"other/deep/est.txt": []byte("in a second directory"),
}
server := httptest.NewServer(fetchTestHandler(manifestOf(t, files), files))
manifest := manifestOf(t, files)
server := httptest.NewServer(fetchTestHandler(manifest, files))
defer server.Close()
dest := chdirTemp(t)
@@ -897,7 +965,9 @@ func TestFetchTree(t *testing.T) {
opts := testOpts([]string{testApp, cmdFetch, "-q", server.URL}, afero.NewOsFs())
require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
assert.Equal(t, files, filesUnder(t, dest))
want := maps.Clone(files)
want[defaultManifestName] = manifest
assert.Equal(t, want, filesUnder(t, dest))
}
// TestFetchFailsOnHashMismatch runs fetch against a server that serves a
@@ -922,9 +992,10 @@ func TestFetchFailsOnHashMismatch(t *testing.T) {
// TestFetchIntoPartlyFilledDestination runs fetch where an interrupted
// fetch of an older version of the tree left one file current, one file
// out of date and one half written to its temp file, beside a file the
// manifest does not list. fetch downloads every file the manifest lists,
// those already present included, and replaces what is there; the file
// the manifest does not list is left alone.
// manifest does not list. fetch skips the current file and downloads the
// other two. The out-of-date file has the same size as the new version,
// so only its hash shows that it must be replaced. The file the manifest
// does not list is left alone, and the manifest is saved beside the files.
//
//nolint:paralleltest // changes the process-global working directory
func TestFetchIntoPartlyFilledDestination(t *testing.T) {
@@ -935,7 +1006,8 @@ func TestFetchIntoPartlyFilledDestination(t *testing.T) {
}
unlisted := []byte("not in the manifest")
tree := fetchTestHandler(manifestOf(t, files), files)
manifest := manifestOf(t, files)
tree := fetchTestHandler(manifest, files)
var (
mu sync.Mutex
@@ -963,21 +1035,208 @@ func TestFetchIntoPartlyFilledDestination(t *testing.T) {
os.WriteFile(tempPathFor("sub/partial.txt"), []byte("cut off"), 0o600))
require.NoError(t, os.WriteFile("unlisted.txt", unlisted, 0o600))
opts := testOpts([]string{testApp, cmdFetch, "-q", server.URL}, afero.NewOsFs())
opts := testOpts([]string{testApp, cmdFetch, server.URL}, afero.NewOsFs())
require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
assert.Contains(t, testStderr(t, opts), "skipping current.txt: already present")
want := maps.Clone(files)
want["unlisted.txt"] = unlisted
want[defaultManifestName] = manifest
assert.Equal(t, want, filesUnder(t, dest))
mu.Lock()
defer mu.Unlock()
assert.ElementsMatch(t, []string{
"/" + defaultManifestName, "/current.txt", "/sub/changed.txt", "/sub/partial.txt",
"/" + defaultManifestName, "/sub/changed.txt", "/sub/partial.txt",
}, requested)
}
// TestFetchIntoDest fetches a tree with --dest into a directory that does
// not exist yet. The files and the manifest must land there and nowhere
// else, and check must pass on the result with no extra files. A second
// fetch into the same directory must download nothing but the manifest.
//
//nolint:paralleltest // changes the process-global working directory
func TestFetchIntoDest(t *testing.T) {
files := map[string][]byte{
"top.txt": []byte("at the top"),
"sub/one.txt": []byte("one level down"),
}
manifest := manifestOf(t, files)
tree := fetchTestHandler(manifest, files)
var (
mu sync.Mutex
requested []string
)
server := httptest.NewServer(
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
mu.Lock()
requested = append(requested, r.URL.Path)
mu.Unlock()
tree.ServeHTTP(w, r)
}))
defer server.Close()
cwd := chdirTemp(t)
dest := filepath.Join(t.TempDir(), "mirror")
fetch := []string{testApp, cmdFetch, "-q", "--" + flagDest, dest, server.URL}
opts := testOpts(fetch, 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))
assert.Empty(t, filesUnder(t, cwd), "fetch wrote outside --dest")
check := testOpts([]string{
testApp, cmdCheck, "-q", testFlagBase, dest, testFlagNoExtra,
filepath.Join(dest, defaultManifestName),
}, afero.NewOsFs())
require.Equal(t, 0, runCLI(check), testStderr(t, check))
mu.Lock()
requested = nil
mu.Unlock()
opts = testOpts(fetch, afero.NewOsFs())
require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
assert.Equal(t, want, filesUnder(t, dest))
mu.Lock()
defer mu.Unlock()
assert.Equal(t, []string{"/" + defaultManifestName}, requested)
}
// TestFetchRequireSignature runs fetch with --require-signature. A
// manifest that is unsigned, or signed by another key, must stop fetch
// with check's message before it downloads or writes anything; the
// required key lets it through. The signed cases need gpg and are skipped
// without it, as the other signing tests are.
//
//nolint:paralleltest // signedManifest calls t.Setenv, which bars t.Parallel
func TestFetchRequireSignature(t *testing.T) {
files := map[string][]byte{testFileTxt: []byte("signed file")}
t.Run("unsigned", func(t *testing.T) {
assertFetchRefused(t, manifestOf(t, files), files,
"manifest is not signed, but signature from "+msgFpA+" is required",
"--"+flagRequireSignature, msgFpA)
})
t.Run("signed", func(t *testing.T) {
manifest := signedManifest(t, files)
signer, err := signedChecker(t, manifest).
ExtractEmbeddedSigningKeyFP(context.Background())
require.NoError(t, err)
assertFetchRefused(t, manifest, files,
"embedded signing key fingerprint "+signer+" does not match required "+msgFpB,
"--"+flagRequireSignature, msgFpB)
server := httptest.NewServer(fetchTestHandler(manifest, files))
defer server.Close()
dest := t.TempDir()
opts := testOpts([]string{
testApp, cmdFetch, "-q", "--" + flagDest, dest,
"--" + flagRequireSignature, signer, server.URL,
}, afero.NewOsFs())
require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
assert.Equal(t, files[testFileTxt], filesUnder(t, dest)[testFileTxt])
})
}
// TestFetchRefusesListedManifestName fetches manifests that list, at the
// top of the tree, the name fetch saves the manifest under or that name's
// temp file: as a file, as a directory, in capitals, and with a leading
// "./". Saving the manifest would replace or remove what is listed there,
// so fetch must refuse the manifest before it creates the destination or
// requests any file.
func TestFetchRefusesListedManifestName(t *testing.T) {
t.Parallel()
for _, listed := range []string{
defaultManifestName,
tempPathFor(defaultManifestName),
defaultManifestName + "/" + testFileTxt,
"INDEX.MF",
"./" + defaultManifestName,
} {
t.Run(listed, func(t *testing.T) {
t.Parallel()
// Built directly rather than scanned, since a scan lists no
// 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
require.NoError(t, builder.Build(context.Background(), &manifest))
assertFetchRefused(t, manifest.Bytes(), map[string][]byte{listed: content},
"manifest lists a file where fetch saves the manifest: "+listed)
})
}
}
// 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
// the directory.
func assertFetchRefused(
t *testing.T, manifest []byte, files map[string][]byte,
message string, flags ...string,
) {
t.Helper()
tree := fetchTestHandler(manifest, files)
var (
mu sync.Mutex
requested []string
)
server := httptest.NewServer(
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
mu.Lock()
requested = append(requested, r.URL.Path)
mu.Unlock()
tree.ServeHTTP(w, r)
}))
defer server.Close()
dest := filepath.Join(t.TempDir(), "mirror")
opts := testOpts(slices.Concat(
[]string{testApp, cmdFetch, "-q", "--" + flagDest, dest}, flags, []string{server.URL},
), afero.NewOsFs())
assert.Equal(t, 1, runCLI(opts))
assert.Contains(t, testStderr(t, opts), message)
assert.NoDirExists(t, dest, "fetch created the destination before refusing")
mu.Lock()
defer mu.Unlock()
assert.Equal(t, []string{"/" + defaultManifestName}, requested)
}
// TestFetchTimeoutFlag runs fetch with --timeout against a server that
// never answers. Without the flag's limit the request would wait forever;
// once fetch gives up on it, the server cancels fetch's context so that
+23 -8
View File
@@ -22,8 +22,10 @@ const (
cmdFetch = "fetch"
cmdVersion = "version"
flagProgress = "progress"
flagTimeout = "timeout"
flagProgress = "progress"
flagTimeout = "timeout"
flagDest = "dest"
flagRequireSignature = "require-signature"
manifestArgsUsage = "[manifest file]"
@@ -142,6 +144,17 @@ func commonFlags() []cli.Flag {
}
}
// requireSignatureFlag returns the --require-signature flag taken by the
// check and fetch subcommands.
func requireSignatureFlag() *cli.StringFlag {
return &cli.StringFlag{
Name: flagRequireSignature,
Aliases: []string{"S"},
Usage: "Require manifest to be signed by the specified GPG key ID",
EnvVars: []string{"MFER_REQUIRE_SIGNATURE"},
}
}
func (mfa *CLIApp) generateCommand() *cli.Command {
return &cli.Command{
Name: cmdGenerate,
@@ -229,12 +242,7 @@ func (mfa *CLIApp) checkCommand() *cli.Command {
Name: "no-extra-files",
Usage: "Fail, instead of warning, if files in base directory are not in manifest",
},
&cli.StringFlag{
Name: "require-signature",
Aliases: []string{"S"},
Usage: "Require manifest to be signed by the specified GPG key ID",
EnvVars: []string{"MFER_REQUIRE_SIGNATURE"},
},
requireSignatureFlag(),
),
}
}
@@ -354,6 +362,13 @@ func (mfa *CLIApp) fetchCommand() *cli.Command {
Usage: "Time limit for each HTTP request, including the download " +
"of its body",
},
&cli.StringFlag{
Name: flagDest,
Aliases: []string{"d"},
Value: ".",
Usage: "Directory to download the files and the manifest into",
},
requireSignatureFlag(),
),
}
}