Never report index.mf at the top of the base as unlisted in check (closes #229)

gen and fetch write the manifest as index.mf at the top of the tree.
check left out only the manifest file it was given, so given a URL, or a
manifest stored elsewhere, it reported that index.mf as not in the
manifest, and --no-extra-files failed a tree fetch had just verified.
check now drops that one report for index.mf at the top of the base
directory. A manifest that lists index.mf still has it checked, and
index.mf in a subdirectory or fetch's leftover .index.mf.tmp are still
reported. The change is in package cli, which owns the default manifest
name; the Checker in package mfer is unchanged.

Model: opus-5-5

docs/FORMAT.md

@@ -93,15 +93,16 @@ Each file entry contains:
 ## File Mode
 
 `mode` holds a file's Unix permission bits, the nine `rwx` bits, so it is never
-above `0777` (octal); the setuid, setgid and sticky bits are never recorded. A
-reader must reject a manifest with an entry whose mode is above `0777`. Writers
-record `0000` unless whoever creates the manifest asks for permissions. `0000`,
-the proto3 default, means no mode was recorded: readers never check or apply it.
+above `0777` (octal); the setuid, setgid and sticky bits are never recorded.
+Writers record `0000` unless whoever creates the manifest asks for permissions.
+`0000`, the proto3 default, means no mode was recorded: readers never check or
+apply it.
 
 The reference implementation records modes when `gen` or `freshen` is given
 `--include-permissions`. `check` fails a file whose permission bits differ from
 a recorded mode other than `0000`. `fetch` sets a recorded mode other than
-`0000` on each file it writes.
+`0000` on each file it writes, and refuses a manifest that records a mode above
+`0777` before it requests any file.
 
 ## Path Rules
 

internal/cli/fetch.go

@@ -95,6 +95,9 @@ var (
 	// writes another file.
 	errNameClash = errors.New(
 		"manifest lists a file where fetch writes another file")
+	// errModeOutOfRange indicates a manifest that lists a mode with more
+	// than the permission bits, such as setuid.
+	errModeOutOfRange = errors.New("manifest lists a mode outside 0777")
 )
 
 // DownloadProgress reports the progress of a single file download.
@@ -425,9 +428,9 @@ func (mfa *CLIApp) fetchManifestOperation(
 
 // 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 fetch writes another. It returns the manifest as
-// downloaded, to be saved once the files are in place, and the files it
-// lists.
+// that lists a file where fetch writes another or a mode outside 0777. It
+// returns the manifest as downloaded, to be saved once the files are in
+// place, and the files it lists.
 func (mfa *CLIApp) fetchManifest(
 	ctx context.Context, cmd *cli.Command, client retryingClient, manifestURL string,
 ) ([]byte, []*mfer.MFFilePath, error) {
@@ -484,6 +487,16 @@ func (mfa *CLIApp) fetchManifest(
 		return nil, nil, err
 	}
 
+	// fetch sets each recorded mode on the file it writes, so a mode above
+	// 0777, which could carry setuid, setgid or sticky bits, is refused
+	// before any file is requested.
+	for _, f := range files {
+		if f.GetMode() > uint32(os.ModePerm) {
+			return nil, nil, fmt.Errorf("%w: %s (%#o)",
+				errModeOutOfRange, f.GetPath(), f.GetMode())
+		}
+	}
+
 	log.Infof("manifest contains %d files", len(files))
 
 	return manifestData, files, nil

internal/cli/fetch_test.go

@@ -19,12 +19,15 @@ import (
 	"sync/atomic"
 	"testing"
 	"time"
+	"uuid"
 
+	"github.com/klauspost/compress/zstd"
 	"github.com/multiformats/go-multihash"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	urfcli "github.com/urfave/cli/v3"
+	"google.golang.org/protobuf/proto"
 	"sneak.berlin/go/mfer/mfer"
 )
 
@@ -1532,9 +1535,27 @@ func TestFetchSetsRecordedMode(t *testing.T) {
 		[]string{"/" + defaultManifestName, "/" + testFileTxt}, requested)
 }
 
+// TestFetchRefusesModeOutsidePermissionBits fetches manifests that record
+// a mode with the setuid bit, once as Unix writes it (04755) and once as
+// Go keeps it in os.FileMode. fetch must refuse both before it creates the
+// destination or requests any file.
+func TestFetchRefusesModeOutsidePermissionBits(t *testing.T) {
+	t.Parallel()
+
+	files := map[string][]byte{testFileTxt: []byte("a file")}
+
+	assertFetchRefused(t, manifestWithMode(t, testFileTxt, files[testFileTxt], 0o4755),
+		files, "manifest lists a mode outside 0777: file.txt (04755)")
+
+	assertFetchRefused(t,
+		manifestWithMode(t, testFileTxt, files[testFileTxt],
+			uint32(os.ModeSetuid|0o755)),
+		files, "manifest lists a mode outside 0777: file.txt (040000755)")
+}
+
 // TestDownloadFileSetsOnlyPermissionBits downloads a file whose entry
 // records mode 0755 together with setuid, setgid or sticky, as Go keeps
-// them in os.FileMode. Loading a manifest refuses such an entry; called
+// them in os.FileMode. fetchManifest would refuse such an entry; called
 // directly, downloadFile must still set only the permission bits.
 func TestDownloadFileSetsOnlyPermissionBits(t *testing.T) {
 	t.Parallel()
@@ -1571,6 +1592,51 @@ func TestDownloadFileSetsOnlyPermissionBits(t *testing.T) {
 	}
 }
 
+// manifestWithMode returns a manifest listing one file, path, with content
+// and the given recorded mode. It is assembled by hand, since the builder
+// never records a mode outside 0777.
+func manifestWithMode(t *testing.T, path string, content []byte, mode uint32) []byte {
+	t.Helper()
+
+	digest := sha256.Sum256(content)
+	hash, err := multihash.Encode(digest[:], multihash.SHA2_256)
+	require.NoError(t, err)
+
+	id := uuid.NewV4()
+
+	inner, err := proto.Marshal(&mfer.MFFile{
+		Version: mfer.MFFile_VERSION_ONE,
+		Files: []*mfer.MFFilePath{{
+			Path:   path,
+			Size:   int64(len(content)),
+			Hashes: []*mfer.MFFileChecksum{{MultiHash: hash}},
+			Mode:   mode,
+		}},
+		Uuid: id[:],
+	})
+	require.NoError(t, err)
+
+	encoder, err := zstd.NewWriter(nil)
+	require.NoError(t, err)
+
+	compressed := encoder.EncodeAll(inner, nil)
+	require.NoError(t, encoder.Close())
+
+	sum := sha256.Sum256(compressed)
+
+	outer, err := proto.Marshal(&mfer.MFFileOuter{
+		Version:         mfer.MFFileOuter_VERSION_ONE,
+		CompressionType: mfer.MFFileOuter_COMPRESSION_ZSTD,
+		Size:            int64(len(inner)),
+		Sha256:          sum[:],
+		Uuid:            id[:],
+		InnerMessage:    compressed,
+	})
+	require.NoError(t, err)
+
+	return append([]byte(mfer.MAGIC), outer...)
+}
+
 // builtManifest returns a manifest of files, built directly rather than
 // scanned, since a scan lists no hidden files.
 func builtManifest(t *testing.T, files map[string][]byte) []byte {

mfer/deserialize.go

@@ -6,7 +6,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"os"
 	"strings"
 
 	"github.com/klauspost/compress/zstd"
@@ -30,7 +29,6 @@ var (
 	errInvalidManifestPath  = errors.New("invalid file entry")
 	errEntryMissingHash     = errors.New("file entry has no SHA-256 hash")
 	errEntryExtraSHA256Hash = errors.New("file entry has more than one SHA-256 hash")
-	errModeOutOfRange       = errors.New("file entry has a mode above 0777")
 	errDecodedTooLarge      = errors.New("too much memory needed")
 	errSignerNotSigningKey  = errors.New(
 		"signer is not the fingerprint of the key that made the signature")
@@ -322,9 +320,7 @@ func (m *manifest) deserializeInner() error {
 	// dropping entries, which would let a hostile manifest hide files from a
 	// check. A path listed twice is refused too: check would check the one
 	// file against both entries. So is an entry that does not list exactly
-	// one SHA-256 hash, and one that records a mode above 0777, such as one
-	// with setuid: a mode holds the permission bits alone, which are all
-	// check compares and fetch sets.
+	// one SHA-256 hash.
 	seen := make(map[string]bool, len(m.pbInner.GetFiles()))
 
 	for _, f := range m.pbInner.GetFiles() {
@@ -338,10 +334,6 @@ func (m *manifest) deserializeInner() error {
 			return err
 		}
 
-		if f.GetMode() > uint32(os.ModePerm) {
-			return fmt.Errorf("%w: %q (%#o)", errModeOutOfRange, f.GetPath(), f.GetMode())
-		}
-
 		if seen[f.GetPath()] {
 			return fmt.Errorf("%w %q", errDuplicatePath, f.GetPath())
 		}

mfer/deserialize_path_test.go

@@ -7,7 +7,6 @@ import (
 	"crypto/sha256"
 	"crypto/sha512"
 	"fmt"
-	"os"
 	"strconv"
 	"strings"
 	"testing"
@@ -235,66 +234,6 @@ func TestDeserializeRequiresExactlyOneSHA256Hash(t *testing.T) {
 	}
 }
 
-// A file entry that records a mode with more than the permission bits is
-// refused as it is loaded, naming the path and the mode: once with setuid as
-// Unix writes it (04755) and once as Go keeps it in os.FileMode. An entry
-// recording 0777 loads.
-func TestDeserializeRefusesModeOutsidePermissionBits(t *testing.T) {
-	t.Parallel()
-
-	hash, err := multihash.Encode(make([]byte, sha256.Size), multihash.SHA2_256)
-	require.NoError(t, err)
-
-	tests := []struct {
-		name string
-		mode uint32
-		want string // "" for an entry that loads
-	}{
-		{
-			"setuid as Unix writes it",
-			0o4755,
-			`file entry has a mode above 0777: "x" (04755)`,
-		},
-		{
-			"setuid as Go keeps it",
-			uint32(os.ModeSetuid | 0o755),
-			`file entry has a mode above 0777: "x" (040000755)`,
-		},
-		{"permission bits only", 0o777, ""},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			id := uuid.NewV4()
-			inner, err := proto.Marshal(&MFFile{
-				Version: MFFile_VERSION_ONE,
-				Files: []*MFFilePath{{
-					Path:   "x",
-					Size:   1,
-					Hashes: []*MFFileChecksum{{MultiHash: hash}},
-					Mode:   tt.mode,
-				}},
-				Uuid: id[:],
-			})
-			require.NoError(t, err)
-
-			m, err := NewManifestFromReader(bytes.NewReader(wrapInner(t, id, inner)))
-			if tt.want == "" {
-				require.NoError(t, err)
-				require.Len(t, m.Files(), 1)
-				assert.Equal(t, tt.mode, m.Files()[0].GetMode())
-
-				return
-			}
-
-			require.ErrorIs(t, err, errModeOutOfRange)
-			require.EqualError(t, err, tt.want)
-		})
-	}
-}
-
 // Entries of a path, a SHA-256 hash, an empty MIME type and empty
 // modification and change times are counted at 432 bytes each (176 + 112 +
 // 16 + 64 + 64) and take 52 bytes plus the path to encode. A one-character