Require a SHA-256 hash in every manifest entry (closes #224)

Loading accepted an entry whose hashes held no SHA-256 multihash, and
check and fetch, which compute only SHA-256, then reported the intact
file as a hash mismatch. Loading now refuses, naming the path, an entry
that does not list exactly one SHA-256 multihash; other hashes beside
it are allowed. AddFileWithHash accepts only a SHA-256 multihash, and
freshen carries an unchanged entry's SHA-256 hash over instead of its
first hash, found with SHA256Multihash, new in package mfer.
docs/FORMAT.md states the rule for writers and readers. Two loading
tests whose entries held an empty hash now give each entry a SHA-256
hash; the decoding-size test's path lengths change with it.

Model: opus-5-5

docs/FORMAT.md

@@ -80,15 +80,15 @@ After decompressing `innerMessage`, the result is a serialized `MFFile`
 
 Each file entry contains:
 
-| Field      | Number | Type                      | Description                                        |
+| Field      | Number | Type                      | Description                                                  |
-| ---------- | ------ | ------------------------- | -------------------------------------------------- |
+| ---------- | ------ | ------------------------- | ------------------------------------------------------------ |
-| `path`     | 1      | string                    | Relative file path (see Path Rules)                |
+| `path`     | 1      | string                    | Relative file path (see Path Rules)                          |
-| `size`     | 2      | int64                     | File size in bytes                                 |
+| `size`     | 2      | int64                     | File size in bytes                                           |
-| `hashes`   | 3      | repeated `MFFileChecksum` | Must include a SHA-256 multihash (see Hash Format) |
+| `hashes`   | 3      | repeated `MFFileChecksum` | Must include exactly one SHA-256 multihash (see Hash Format) |
-| `mimeType` | 301    | string (optional)         | MIME type                                          |
+| `mimeType` | 301    | string (optional)         | MIME type                                                    |
-| `mtime`    | 302    | Timestamp (optional)      | Modification time                                  |
+| `mtime`    | 302    | Timestamp (optional)      | Modification time                                            |
-| `ctime`    | 303    | Timestamp (optional)      | Change time (inode metadata change)                |
+| `ctime`    | 303    | Timestamp (optional)      | Change time (inode metadata change)                          |
-| `mode`     | 304    | uint32                    | Permission bits (see File Mode)                    |
+| `mode`     | 304    | uint32                    | Permission bits (see File Mode)                              |
 
 ## File Mode
 
@@ -133,12 +133,13 @@ Each checksum is a single `bytes multiHash` field containing a
 self-describing: the encoded bytes include a varint algorithm identifier
 followed by a varint digest length followed by the digest itself.
 
-A writer must give every file entry a SHA-256 multihash of the file's content:
+A writer must give every file entry exactly one SHA-256 multihash of the file's
-the algorithm code `0x12`, the digest length `0x20` and the 32 bytes of the
+content: the algorithm code `0x12`, the digest length `0x20` and the 32 bytes of
-digest. It may list multihashes of other algorithms beside it. A reader must
+the digest. It may list multihashes of other algorithms beside it. A reader must
-refuse a manifest with an entry that has no SHA-256 multihash, whatever other
+refuse a manifest with an entry that has no SHA-256 multihash or more than one,
-hashes the entry lists. A verifier then needs only SHA-256, checking each file
+whatever other hashes the entry lists. A verifier then needs only SHA-256,
-against its entry's SHA-256 multihash; it may ignore the other hashes.
+checking each file against its entry's SHA-256 multihash; it may ignore the
+other hashes.
 
 The reference implementation writes one hash for each entry, the SHA-256
 multihash, and its `check` and `fetch` compute only SHA-256.

internal/cli/freshen.go

@@ -718,10 +718,7 @@ func addExistingToBuilder(b *mfer.Builder, entry *mfer.MFFilePath) error {
 		mfer.FileSize(entry.GetSize()), mfer.ModTime(mtime),
 		fs.FileMode(entry.GetMode()), hash)
 	if err != nil {
-		return fmt.Errorf(
+		return fmt.Errorf("manifest entry %s: %w", entry.GetPath(), err)
-			"manifest entry %s: %w (regenerate the manifest with mfer generate)",
-			entry.GetPath(), err,
-		)
 	}
 
 	return nil

mfer/builder.go

@@ -113,8 +113,8 @@ func (m ModTime) Timestamp() *Timestamp {
 type Multihash []byte
 
 // SHA256Multihash returns the first of entry's hashes that is a SHA-256
-// multihash, or nil if none is. Loading a manifest refuses an entry
+// multihash, or nil if none is. Loading a manifest refuses an entry that
-// without one.
+// does not list exactly one, so for a loaded entry it is the only one.
 func SHA256Multihash(entry *MFFilePath) Multihash {
 	for _, h := range entry.GetHashes() {
 		if isSHA256Multihash(h.GetMultiHash()) {

mfer/deserialize.go

@@ -28,6 +28,7 @@ var (
 	errInvalidFileFormat    = errors.New("invalid file format")
 	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")
 	errDecodedTooLarge      = errors.New("too much memory needed")
 	errSignerNotSigningKey  = errors.New(
 		"signer is not the fingerprint of the key that made the signature")
@@ -236,6 +237,30 @@ func forEachBytesField(
 	return nil
 }
 
+// validateEntryHashes refuses, naming its path, an entry that does not list
+// exactly one SHA-256 multihash; hashes of other algorithms beside it are
+// allowed. check and fetch compute only SHA-256, so with none they would
+// have nothing to check the file against, and with two check would pass a
+// file that matches either.
+func validateEntryHashes(entry *MFFilePath) error {
+	sha256Hashes := 0
+
+	for _, h := range entry.GetHashes() {
+		if isSHA256Multihash(h.GetMultiHash()) {
+			sha256Hashes++
+		}
+	}
+
+	switch {
+	case sha256Hashes == 0:
+		return fmt.Errorf("%w: %q", errEntryMissingHash, entry.GetPath())
+	case sha256Hashes > 1:
+		return fmt.Errorf("%w: %q", errEntryExtraSHA256Hash, entry.GetPath())
+	default:
+		return nil
+	}
+}
+
 func (m *manifest) deserializeInner() error {
 	err := m.validateOuterHeader()
 	if err != nil {
@@ -294,9 +319,8 @@ func (m *manifest) deserializeInner() error {
 	// untrusted .mf. Reject loudly on the first offender rather than
 	// 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 with no SHA-256 hash, even
+	// file against both entries. So is an entry that does not list exactly
-	// one that lists other hashes: check and fetch compute only SHA-256, so
+	// one SHA-256 hash.
-	// they would have nothing to check the file against.
 	seen := make(map[string]bool, len(m.pbInner.GetFiles()))
 
 	for _, f := range m.pbInner.GetFiles() {
@@ -305,8 +329,9 @@ func (m *manifest) deserializeInner() error {
 			return fmt.Errorf("%w: %w", errInvalidManifestPath, err)
 		}
 
-		if SHA256Multihash(f) == nil {
+		err = validateEntryHashes(f)
-			return fmt.Errorf("%w: %q", errEntryMissingHash, f.GetPath())
+		if err != nil {
+			return err
 		}
 
 		if seen[f.GetPath()] {

mfer/deserialize_path_test.go

@@ -169,39 +169,67 @@ func TestDeserializeRefusesPathListedTwice(t *testing.T) {
 	}
 }
 
-// A manifest holding a file entry with no SHA-256 hash, whether it lists no
+// A file entry must list exactly one SHA-256 hash, and may list hashes of
-// hash or only a SHA-512 one, is refused as it is loaded, naming the path:
+// other algorithms beside it. An entry with none, even one that lists a
-// check and fetch compute only SHA-256, so they would have nothing to check
+// SHA-512 hash, is refused as it is loaded, and so is an entry with two;
-// the file against.
+// each refusal names the path. An entry listing a SHA-512 hash and then a
-func TestDeserializeRefusesEntryWithoutSHA256Hash(t *testing.T) {
+// SHA-256 one loads with both.
+func TestDeserializeRequiresExactlyOneSHA256Hash(t *testing.T) {
 	t.Parallel()
 
+	sha256Hash, err := multihash.Encode(make([]byte, sha256.Size), multihash.SHA2_256)
+	require.NoError(t, err)
+
+	otherSHA256Hash, err := multihash.Encode(
+		bytes.Repeat([]byte{1}, sha256.Size), multihash.SHA2_256)
+	require.NoError(t, err)
+
 	sha512Hash, err := multihash.Encode(make([]byte, sha512.Size), multihash.SHA2_512)
 	require.NoError(t, err)
 
 	tests := []struct {
 		name   string
-		hashes []*MFFileChecksum
+		hashes [][]byte
+		want   error // nil for an entry that loads
 	}{
-		{"no hash", nil},
+		{"no hash", nil, errEntryMissingHash},
-		{"SHA-512 hash only", []*MFFileChecksum{{MultiHash: sha512Hash}}},
+		{"SHA-512 hash only", [][]byte{sha512Hash}, errEntryMissingHash},
+		{
+			"two SHA-256 hashes",
+			[][]byte{sha256Hash, otherSHA256Hash},
+			errEntryExtraSHA256Hash,
+		},
+		{"SHA-512 hash then SHA-256 hash", [][]byte{sha512Hash, sha256Hash}, nil},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
+			entry := &MFFilePath{Path: "x", Size: 1}
+			for _, h := range tt.hashes {
+				entry.Hashes = append(entry.Hashes, &MFFileChecksum{MultiHash: h})
+			}
+
 			id := uuid.NewV4()
 			inner, err := proto.Marshal(&MFFile{
 				Version: MFFile_VERSION_ONE,
-				Files:   []*MFFilePath{{Path: "x", Size: 1, Hashes: tt.hashes}},
+				Files:   []*MFFilePath{entry},
 				Uuid:    id[:],
 			})
 			require.NoError(t, err)
 
-			_, err = NewManifestFromReader(bytes.NewReader(wrapInner(t, id, inner)))
+			m, err := NewManifestFromReader(bytes.NewReader(wrapInner(t, id, inner)))
-			require.ErrorIs(t, err, errEntryMissingHash)
+			if tt.want == nil {
-			require.EqualError(t, err, `file entry has no SHA-256 hash: "x"`)
+				require.NoError(t, err)
+				require.Len(t, m.Files(), 1)
+				assert.Len(t, m.Files()[0].GetHashes(), len(tt.hashes))
+
+				return
+			}
+
+			require.ErrorIs(t, err, tt.want)
+			assert.Contains(t, err.Error(), `"x"`)
 		})
 	}
 }