From 24487ce36aec41f5a6b64b131e9031a64d6e05be Mon Sep 17 00:00:00 2001 From: sneak Date: Wed, 21 Oct 2026 10:16:30 +0000 Subject: [PATCH] Export the loaded manifest type as Manifest (closes #236) Package mfer handed out a loaded manifest as the unexported type manifest, so a program using the library could not name it, keep it in a struct field or pass it to its own functions. As sneak approved, it is now exported as Manifest with its fields kept private, so its existing methods (Files, Signer, CreatedAt, String) are its whole API. NewManifestFromReader and NewManifestFromFile return *Manifest, and the lint suppressions for returning an unexported type are gone. The type's doc comment no longer points at the settled design question, and String has a doc comment. A new test in package mfer_test uses *mfer.Manifest from outside the package. Model: opus-5-5 --- mfer/builder.go | 2 +- mfer/checker.go | 2 +- mfer/deserialize.go | 18 +++++-------- mfer/errmsg_test.go | 2 +- mfer/manifest.go | 23 ++++++++-------- mfer/manifest_external_test.go | 49 ++++++++++++++++++++++++++++++++++ mfer/manifest_test.go | 2 +- mfer/serialize.go | 6 ++--- 8 files changed, 74 insertions(+), 30 deletions(-) create mode 100644 mfer/manifest_external_test.go diff --git a/mfer/builder.go b/mfer/builder.go index 5582ddd..e74a719 100644 --- a/mfer/builder.go +++ b/mfer/builder.go @@ -335,7 +335,7 @@ func (b *Builder) Build(ctx context.Context, w io.Writer) error { } // Create a temporary manifest to use existing serialization - m := &manifest{ + m := &Manifest{ pbInner: inner, signingOptions: b.signingOptions, fixedUUID: b.fixedUUID, diff --git a/mfer/checker.go b/mfer/checker.go index 5184058..37648be 100644 --- a/mfer/checker.go +++ b/mfer/checker.go @@ -123,7 +123,7 @@ func NewChecker(opts *CheckerOptions) (*Checker, error) { } var ( - m *manifest + m *Manifest manifestInfo os.FileInfo err error ) diff --git a/mfer/deserialize.go b/mfer/deserialize.go index d9bc2da..6f83449 100644 --- a/mfer/deserialize.go +++ b/mfer/deserialize.go @@ -48,7 +48,7 @@ func validateUUID(data []byte) error { // validateOuterHeader checks the outer message's version, compression // type, and UUID. -func (m *manifest) validateOuterHeader() error { +func (m *Manifest) validateOuterHeader() error { if m.pbOuter.GetVersion() != MFFileOuter_VERSION_ONE { return errUnknownVersion } @@ -70,7 +70,7 @@ func (m *manifest) validateOuterHeader() error { // signature is present, verifies it against the embedded public key, which // must be one key, and checks that the signer field is that key's // fingerprint. -func (m *manifest) verifyOuterIntegrity() error { +func (m *Manifest) verifyOuterIntegrity() error { h := sha256.New() _, err := h.Write(m.pbOuter.GetInnerMessage()) @@ -117,7 +117,7 @@ func (m *manifest) verifyOuterIntegrity() error { // decompressInner decompresses the inner payload, enforcing size limits // to prevent decompression bombs. -func (m *manifest) decompressInner() ([]byte, error) { +func (m *Manifest) decompressInner() ([]byte, error) { bb := bytes.NewBuffer(m.pbOuter.GetInnerMessage()) // By default the decoder decodes a payload under 128 KiB in full, @@ -259,7 +259,7 @@ func validateEntryHashes(entry *MFFilePath) error { } } -func (m *manifest) deserializeInner() error { +func (m *Manifest) deserializeInner() error { err := m.validateOuterHeader() if err != nil { return err @@ -364,10 +364,8 @@ func validateMagic(dat []byte) bool { // NewManifestFromReader reads a manifest from an io.Reader. It refuses a // manifest larger than MaxManifestSize, reading at most one byte past it. -// -//nolint:revive // unexported-return: exporting manifest is owner question 13 -func NewManifestFromReader(input io.Reader) (*manifest, error) { - m := &manifest{} +func NewManifestFromReader(input io.Reader) (*Manifest, error) { + m := &Manifest{} dat, err := readAtMost(input, MaxManifestSize) if err != nil { @@ -426,9 +424,7 @@ type ManifestFromFileOptions struct { // NewManifestFromFile reads a manifest from a file. It returns an error if // opts is nil or its path is empty. -// -//nolint:revive // unexported-return: exporting manifest is owner question 13 -func NewManifestFromFile(opts *ManifestFromFileOptions) (*manifest, error) { +func NewManifestFromFile(opts *ManifestFromFileOptions) (*Manifest, error) { if opts == nil || opts.Path == "" { return nil, errManifestPathEmpty } diff --git a/mfer/errmsg_test.go b/mfer/errmsg_test.go index bd556c8..abef564 100644 --- a/mfer/errmsg_test.go +++ b/mfer/errmsg_test.go @@ -85,7 +85,7 @@ func TestValidatePathMessagesVerbatim(t *testing.T) { func TestSerializeInnerNotSetMessagesVerbatim(t *testing.T) { t.Parallel() - m := &manifest{} + m := &Manifest{} err := m.generate(context.Background()) require.ErrorIs(t, err, errInnerNotSet) diff --git a/mfer/manifest.go b/mfer/manifest.go index 2730612..634279c 100644 --- a/mfer/manifest.go +++ b/mfer/manifest.go @@ -15,13 +15,10 @@ var ( errSHA256NotSet = errors.New("SHA256 hash not set") ) -// manifest holds the internal representation of a manifest file. -// Use NewManifestFromFile or NewManifestFromReader to load an existing -// manifest, or use Builder to create a new one. -// -// Whether this type should be exported is an open design question owned by -// the repository owner; see README design question 13. -type manifest struct { +// Manifest is a manifest loaded by NewManifestFromFile or +// NewManifestFromReader, or made by Builder to write a new manifest file. +// Its fields are private; only its methods are the library's API. +type Manifest struct { pbInner *MFFile pbOuter *MFFileOuter output *bytes.Buffer @@ -29,7 +26,9 @@ type manifest struct { fixedUUID []byte // if set, use this UUID instead of generating one } -func (m *manifest) String() string { +// String returns a short description giving the number of files the +// manifest lists, such as "". +func (m *Manifest) String() string { count := 0 if m.pbInner != nil { count = len(m.pbInner.GetFiles()) @@ -39,7 +38,7 @@ func (m *manifest) String() string { } // Files returns all file entries from a loaded manifest. -func (m *manifest) Files() []*MFFilePath { +func (m *Manifest) Files() []*MFFilePath { if m.pbInner == nil { return nil } @@ -50,7 +49,7 @@ func (m *manifest) Files() []*MFFilePath { // Signer returns the fingerprint of the key that made a loaded manifest's // signature, which loading the manifest checked, or nil if the manifest is // not signed. -func (m *manifest) Signer() []byte { +func (m *Manifest) Signer() []byte { if len(m.pbOuter.GetSignature()) == 0 { return nil } @@ -60,14 +59,14 @@ func (m *manifest) Signer() []byte { // CreatedAt returns the createdAt timestamp of a loaded manifest, or nil // if the manifest has none. -func (m *manifest) CreatedAt() *Timestamp { +func (m *Manifest) CreatedAt() *Timestamp { return m.pbInner.GetCreatedAt() } // signatureString generates the canonical string used for signing/verification. // Format: MAGIC-UUID-MULTIHASH where UUID and multihash are hex-encoded. // Requires pbOuter to be set with Uuid and Sha256 fields. -func (m *manifest) signatureString() (string, error) { +func (m *Manifest) signatureString() (string, error) { if m.pbOuter == nil { return "", errOuterNotSet } diff --git a/mfer/manifest_external_test.go b/mfer/manifest_external_test.go new file mode 100644 index 0000000..28a8d67 --- /dev/null +++ b/mfer/manifest_external_test.go @@ -0,0 +1,49 @@ +package mfer_test + +import ( + "bytes" + "context" + "testing" + + "github.com/stretchr/testify/require" + "sneak.berlin/go/mfer/mfer" +) + +// archive keeps a loaded manifest in a field, as a type in a program that +// imports mfer would. +type archive struct { + manifest *mfer.Manifest +} + +// fileCount takes a loaded manifest, as a function in such a program would. +func fileCount(m *mfer.Manifest) int { + return len(m.Files()) +} + +// A program that imports mfer can name the type of a loaded manifest, keep +// one in a struct field, pass it to its own functions, and read it through +// its methods. +func TestManifestIsUsableOutsideThePackage(t *testing.T) { + t.Parallel() + + content := []byte("hello\n") + builder := mfer.NewBuilder() + + _, err := builder.AddFile("hello.txt", mfer.FileSize(len(content)), + mfer.ModTime{}, 0, bytes.NewReader(content), nil) + require.NoError(t, err) + + var written bytes.Buffer + require.NoError(t, builder.Build(context.Background(), &written)) + + loaded, err := mfer.NewManifestFromReader(&written) + require.NoError(t, err) + + a := archive{manifest: loaded} + + require.Equal(t, 1, fileCount(a.manifest)) + require.Equal(t, "hello.txt", a.manifest.Files()[0].GetPath()) + require.Nil(t, a.manifest.Signer()) + require.Nil(t, a.manifest.CreatedAt()) + require.Equal(t, "", a.manifest.String()) +} diff --git a/mfer/manifest_test.go b/mfer/manifest_test.go index 2b58897..e26a1a2 100644 --- a/mfer/manifest_test.go +++ b/mfer/manifest_test.go @@ -21,7 +21,7 @@ func TestSignatureString(t *testing.T) { "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855") require.NoError(t, err) - m := &manifest{pbOuter: &MFFileOuter{Uuid: manifestUUID[:], Sha256: sha256Hash}} + m := &Manifest{pbOuter: &MFFileOuter{Uuid: manifestUUID[:], Sha256: sha256Hash}} got, err := m.signatureString() require.NoError(t, err) diff --git a/mfer/serialize.go b/mfer/serialize.go index f40eac8..1f10750 100644 --- a/mfer/serialize.go +++ b/mfer/serialize.go @@ -51,7 +51,7 @@ func newTimestampFromTime(t time.Time) *Timestamp { } } -func (m *manifest) generate(ctx context.Context) error { +func (m *Manifest) generate(ctx context.Context) error { if m.pbInner == nil { return errInnerNotSet } @@ -78,7 +78,7 @@ func (m *manifest) generate(ctx context.Context) error { return nil } -func (m *manifest) generateOuter(ctx context.Context) error { +func (m *Manifest) generateOuter(ctx context.Context) error { if m.pbInner == nil { return errInternal } @@ -145,7 +145,7 @@ func (m *manifest) generateOuter(ctx context.Context) error { // signOuter signs the outer message with the secret key in the signing // options and embeds the signature, the key's fingerprint and its public // key. -func (m *manifest) signOuter(ctx context.Context) error { +func (m *Manifest) signOuter(ctx context.Context) error { // Unlocking a protected key can take a while; do not start once ctx // has ended. err := ctx.Err()