Require a SHA-256 hash in every manifest entry (closes #224)
check / check (push) Waiting to run
check / check (push) Waiting to run
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
This commit is contained in:
+11
-11
@@ -33,9 +33,9 @@ var (
|
||||
// modification time where one is required to carry it forward unchanged.
|
||||
errEntryMissingMtime = errors.New("manifest entry has no mtime")
|
||||
|
||||
// errEntryMissingHash indicates a manifest entry that carries no hash
|
||||
// to carry forward.
|
||||
errEntryMissingHash = errors.New("manifest entry has no hash")
|
||||
// errEntryMissingHash indicates a manifest entry that carries no
|
||||
// SHA-256 hash to carry forward.
|
||||
errEntryMissingHash = errors.New("manifest entry has no SHA-256 hash")
|
||||
)
|
||||
|
||||
// errSignKeyNeeded indicates a signed manifest given to freshen with no
|
||||
@@ -699,26 +699,26 @@ func addFileToBuilder(
|
||||
// Entries reach this path only when recordEntry classified them as
|
||||
// unchanged, which requires a recorded mtime, so an absent mtime here is
|
||||
// an error rather than something to paper over with the Unix epoch.
|
||||
// Loading refuses an entry with no hash; one that got here anyway is an
|
||||
// error too, not a file to leave out of the manifest.
|
||||
// Only the entry's SHA-256 hash is carried over, wherever the entry lists
|
||||
// it, since check and fetch compare with no other. Loading refuses an
|
||||
// entry with no SHA-256 hash; one that got here anyway is an error too,
|
||||
// not a file to leave out of the manifest.
|
||||
func addExistingToBuilder(b *mfer.Builder, entry *mfer.MFFilePath) error {
|
||||
mtime, ok := entryMtime(entry)
|
||||
if !ok {
|
||||
return fmt.Errorf("%w: %s", errEntryMissingMtime, entry.GetPath())
|
||||
}
|
||||
|
||||
if len(entry.GetHashes()) == 0 {
|
||||
hash := mfer.SHA256Multihash(entry)
|
||||
if hash == nil {
|
||||
return fmt.Errorf("%w: %s", errEntryMissingHash, entry.GetPath())
|
||||
}
|
||||
|
||||
err := b.AddFileWithHash(mfer.RelFilePath(entry.GetPath()),
|
||||
mfer.FileSize(entry.GetSize()), mfer.ModTime(mtime),
|
||||
fs.FileMode(entry.GetMode()), entry.GetHashes()[0].GetMultiHash())
|
||||
fs.FileMode(entry.GetMode()), hash)
|
||||
if err != nil {
|
||||
return fmt.Errorf(
|
||||
"manifest entry %s: %w (regenerate the manifest with mfer generate)",
|
||||
entry.GetPath(), err,
|
||||
)
|
||||
return fmt.Errorf("manifest entry %s: %w", entry.GetPath(), err)
|
||||
}
|
||||
|
||||
return nil
|
||||
|
||||
@@ -2,7 +2,10 @@
|
||||
package cli
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"crypto/sha256"
|
||||
"crypto/sha512"
|
||||
"maps"
|
||||
"os"
|
||||
"path/filepath"
|
||||
@@ -584,48 +587,76 @@ func TestFreshenAddExistingRejectsMissingMtime(t *testing.T) {
|
||||
assert.Contains(t, err.Error(), "file1.txt")
|
||||
}
|
||||
|
||||
// TestFreshenAddExistingRejectsMissingHash pins that an entry with no hash
|
||||
// is an error rather than a file left out of the rebuilt manifest.
|
||||
// TestFreshenAddExistingRejectsMissingHash pins that an entry with no
|
||||
// SHA-256 hash, whether it lists no hash or only a SHA-1 one, is an error
|
||||
// rather than a file left out of the rebuilt manifest.
|
||||
func TestFreshenAddExistingRejectsMissingHash(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
b := mfer.NewBuilder()
|
||||
entry := &mfer.MFFilePath{
|
||||
Path: "data.bin",
|
||||
Size: 8,
|
||||
Mtime: &mfer.Timestamp{Seconds: 1_700_000_000},
|
||||
}
|
||||
|
||||
err := addExistingToBuilder(b, entry)
|
||||
require.ErrorIs(t, err, errEntryMissingHash)
|
||||
assert.Contains(t, err.Error(), "data.bin")
|
||||
assert.Zero(t, b.FileCount())
|
||||
}
|
||||
|
||||
// TestFreshenAddExistingRejectsShortHash pins that an existing manifest
|
||||
// entry whose hash the builder refuses is reported as a problem with the
|
||||
// manifest, with the command that fixes it.
|
||||
func TestFreshenAddExistingRejectsShortHash(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
sha1Hash, err := multihash.Encode(make([]byte, 20), multihash.SHA1)
|
||||
require.NoError(t, err)
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
hashes []*mfer.MFFileChecksum
|
||||
}{
|
||||
{"no hash", nil},
|
||||
{"SHA-1 hash only", []*mfer.MFFileChecksum{{MultiHash: sha1Hash}}},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
b := mfer.NewBuilder()
|
||||
entry := &mfer.MFFilePath{
|
||||
Path: "data.bin",
|
||||
Size: 8,
|
||||
Mtime: &mfer.Timestamp{Seconds: 1_700_000_000},
|
||||
Hashes: tt.hashes,
|
||||
}
|
||||
|
||||
err := addExistingToBuilder(b, entry)
|
||||
require.ErrorIs(t, err, errEntryMissingHash)
|
||||
assert.Contains(t, err.Error(), "data.bin")
|
||||
assert.Zero(t, b.FileCount())
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestFreshenAddExistingKeepsSHA256Hash pins that an unchanged entry that
|
||||
// lists another hash before its SHA-256 hash is carried over with the
|
||||
// SHA-256 hash, the one check and fetch compare with, and nothing else.
|
||||
func TestFreshenAddExistingKeepsSHA256Hash(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
sha512Hash, err := multihash.Encode(make([]byte, sha512.Size), multihash.SHA2_512)
|
||||
require.NoError(t, err)
|
||||
|
||||
sha256Hash, err := multihash.Encode(make([]byte, sha256.Size), multihash.SHA2_256)
|
||||
require.NoError(t, err)
|
||||
|
||||
b := mfer.NewBuilder()
|
||||
entry := &mfer.MFFilePath{
|
||||
Path: "old.txt",
|
||||
Size: 8,
|
||||
Path: "kept.txt",
|
||||
Size: 6,
|
||||
Mtime: &mfer.Timestamp{Seconds: 1_700_000_000},
|
||||
Hashes: []*mfer.MFFileChecksum{
|
||||
{MultiHash: sha1Hash},
|
||||
{MultiHash: sha512Hash},
|
||||
{MultiHash: sha256Hash},
|
||||
},
|
||||
}
|
||||
|
||||
err = addExistingToBuilder(b, entry)
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "manifest entry old.txt")
|
||||
assert.Contains(t, err.Error(), "mfer generate")
|
||||
assert.Zero(t, b.FileCount())
|
||||
require.NoError(t, addExistingToBuilder(b, entry))
|
||||
|
||||
var buf bytes.Buffer
|
||||
require.NoError(t, b.Build(context.Background(), &buf))
|
||||
|
||||
m, err := mfer.NewManifestFromReader(&buf)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, m.Files(), 1)
|
||||
require.Len(t, m.Files()[0].GetHashes(), 1)
|
||||
assert.Equal(t, sha256Hash, m.Files()[0].GetHashes()[0].GetMultiHash())
|
||||
}
|
||||
|
||||
// TestEntryMtime pins the presence semantics the callers depend on.
|
||||
|
||||
Reference in New Issue
Block a user