Describe the signature scheme in docs/FORMAT.md as the code builds it (closes #187)

The Signature Scheme section now states what signatureString builds: the
magic, the outer uuid as lower-case hex with no hyphens, and the outer
sha256 as the lower-case hex of its multihash, which starts 1220, with an
example string taken from a run of the code. It says signer is the
upper-case hex fingerprint of the signing primary key and that a signature
is stored only in the manifest. The mf.proto comments on fields 201 to 203
say the same, and mf.pb.go is regenerated. A new test pins the example
string. Nothing about what is signed or how changes.

Model: opus-5-5

docs/FORMAT.md

@@ -36,8 +36,8 @@ The outer message contains:
 | `sha256`          | 104    | bytes            | SHA-256 hash of the **compressed** `innerMessage` (corruption detection) |
 | `uuid`            | 105    | bytes            | Random v4 UUID; must match the inner message UUID                        |
 | `innerMessage`    | 199    | bytes            | Zstd-compressed serialized `MFFile` message                              |
-| `signature`       | 201    | bytes (optional) | OpenPGP detached signature (ASCII-armored or binary)                     |
-| `signer`          | 202    | bytes (optional) | Fingerprint of the signing key                                           |
+| `signature`       | 201    | bytes (optional) | One OpenPGP signature over the canonical string, armored or binary       |
+| `signer`          | 202    | bytes (optional) | Fingerprint of the signing primary key, in upper-case hex                |
 | `signingPubKey`   | 203    | bytes (optional) | Full OpenPGP public key of the signing key (ASCII-armored or binary)     |
 
 ### SHA-256 Hash
@@ -128,34 +128,51 @@ Implementations must be able to verify SHA-256 multihashes at minimum.
 
 ## Signature Scheme
 
-Signing is optional. When present, the signature covers a canonical string
-constructed as:
+Signing is optional. A signature is stored only in the outer message's
+`signature` field, never in a separate file. It is one OpenPGP signature over
+the canonical string below, armored or binary, and does not contain the string,
+which a verifier builds again from the outer message.
+
+The canonical string is three parts separated by hyphens (`-`):
 
 ```
 ZNAVSRFG-<UUID>-<SHA256>
 ```
 
-Where:
+- `ZNAVSRFG` is the magic, in ASCII.
+- `<UUID>` is the 16 bytes of the outer message's `uuid` as 32 lower-case hex
+  digits, with no hyphens.
+- `<SHA256>` is the lower-case hex of the outer message's `sha256` encoded as a
+  SHA-256 multihash (see Hash Format). That multihash is the algorithm code
+  `0x12`, the digest length `0x20` and the 32 bytes of the hash, so this part is
+  68 hex digits starting `1220`.
 
-- `ZNAVSRFG` is the magic bytes string (literal ASCII)
-- `<UUID>` is the hex-encoded UUID from the outer message
-- `<SHA256>` is the hex-encoded SHA-256 hash from the outer message (covering
-  compressed data)
+The signature covers exactly the 110 ASCII characters of the string, with no
+line ending. For example, the UUID `b0fb6918-6298-443b-ada2-1b162f4dc0b3` and a
+`sha256` of `e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855`
+in hex give this canonical string:
 
-Components are separated by hyphens. The signature is an OpenPGP detached
-signature over this canonical string, stored in the `signature` field of the
-outer message. The signing key's public key goes in `signingPubKey` and its
-fingerprint, in hex, in `signer`.
+```
+ZNAVSRFG-b0fb69186298443bada21b162f4dc0b3-1220e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
+```
+
+The signature covers `innerMessage` only through `sha256`, so a verifier checks
+`sha256` against `innerMessage` (see SHA-256 Hash) as well as the signature.
+
+The signing key's public key goes in `signingPubKey` and the fingerprint of its
+primary key, in upper-case hex, in `signer`. The reference implementation signs
+only with version 4 keys, whose fingerprints are 40 hex digits, and writes both
+the signature and the public key armored.
 
 A verifier accepts a signed manifest only if `signingPubKey` holds exactly one
 primary key, `signature` is one good signature over the canonical string made by
-that key (or one of its subkeys), and `signer` is that key's fingerprint. The
-reference implementation refuses to load a manifest that fails these checks;
-`check` and `fetch` given `--require-signature` then compare the required
-fingerprint with `signer`. It also refuses a manifest whose `signingPubKey`
-holds a DSA key or subkey, or any secret key or subkey, since checking the
-self-signatures of a DSA key or the numbers of a secret key can take hours when
-those numbers are very large.
+that key (or one of its subkeys), and `signer` is that primary key's
+fingerprint, compared without regard to case. The reference implementation
+refuses to load a manifest that fails these checks; `check` and `fetch` given
+`--require-signature` then compare the required fingerprint with `signer`. It
+also refuses a manifest whose `signingPubKey` holds a DSA key or subkey, or any
+secret key or subkey, since checking the self-signatures of a DSA key or the
+numbers of a secret key can take hours when those numbers are very large.
 
 ## Deterministic Serialization
 

internal/cli/entry_test.go

@@ -1664,20 +1664,119 @@ func TestGenerateValidatesInputPaths(t *testing.T) {
 	})
 }
 
-// TestFlagAfterArgumentIsArgument asserts that flags are read only before a
-// command's first argument: after it, -v is a path, not the verbose flag.
-func TestFlagAfterArgumentIsArgument(t *testing.T) {
-	t.Parallel()
+// generateTestDir writes testFile1 and runs gen on testDir, which writes
+// the manifest to testDir/index.mf.
+func generateTestDir(t *testing.T, fs afero.Fs) {
+	t.Helper()
 
-	fs := afero.NewMemMapFs()
 	require.NoError(t, fs.MkdirAll(testDir, 0o755))
 	writeTestFile(t, fs, testFile1, "content")
 
-	opts := testOpts([]string{testApp, cmdGenerate, testDir, "-v"}, fs)
-	exitCode := runCLI(opts)
+	opts := testOpts([]string{testApp, cmdGenerate, "-q", testDir}, fs)
+	require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
+}
 
-	assert.Equal(t, 1, exitCode)
-	assert.Contains(t, testStderr(t, opts), "path does not exist: -v")
+// TestCheckReadsOptionAfterPath runs check on an unsigned manifest with
+// --require-signature after the directory. check must refuse the manifest.
+func TestCheckReadsOptionAfterPath(t *testing.T) {
+	t.Parallel()
+
+	fs := afero.NewMemMapFs()
+	generateTestDir(t, fs)
+
+	opts := testOpts([]string{
+		testApp, cmdCheck, "-q", testDir, "--" + flagRequireSignature, msgFpA,
+	}, fs)
+	assert.Equal(t, 1, runCLI(opts))
+	assert.Contains(t, testStderr(t, opts), "manifest is not signed")
+}
+
+// TestFreshenReadsOptionAfterPath runs freshen with --include-permissions
+// after the directory. freshen must record the file's mode.
+func TestFreshenReadsOptionAfterPath(t *testing.T) {
+	t.Parallel()
+
+	fs := afero.NewOsFs()
+	root, manifestPath := setupFreshenDir(t, fs,
+		map[string]string{testFileTxt: "a file"})
+	require.NoError(t, fs.Chmod(filepath.Join(root, testFileTxt), 0o640))
+
+	opts := testOpts([]string{
+		testApp, cmdFreshen, "-q", root, "--" + flagIncludePermissions,
+	}, fs)
+	require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
+
+	files := manifestFiles(t, fs, manifestPath)
+	require.Len(t, files, 1)
+	assert.Equal(t, uint32(0o640), files[0].GetMode())
+}
+
+// TestFetchReadsOptionAfterURL runs fetch with --dest after the URL. The
+// file and the manifest must land in that directory and nothing in the
+// current one.
+//
+//nolint:paralleltest // changes the process-global working directory
+func TestFetchReadsOptionAfterURL(t *testing.T) {
+	content := []byte("fetched")
+	manifest := manifestOf(t, map[string][]byte{testFileTxt: content})
+
+	server := httptest.NewServer(fetchTestHandler(manifest,
+		map[string][]byte{testFileTxt: content}))
+	defer server.Close()
+
+	cwd := chdirTemp(t)
+	dest := t.TempDir()
+
+	opts := testOpts([]string{
+		testApp, cmdFetch, "-q", server.URL, "--" + flagDest, dest,
+	}, afero.NewOsFs())
+	require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
+
+	assert.Equal(t,
+		map[string][]byte{testFileTxt: content, defaultManifestName: manifest},
+		filesUnder(t, dest))
+	assert.Empty(t, filesUnder(t, cwd), "fetch wrote outside --dest")
+}
+
+// TestOneArgumentCommandsRefuseSecond runs each command that takes one
+// argument with two. It must fail naming the second.
+func TestOneArgumentCommandsRefuseSecond(t *testing.T) {
+	t.Parallel()
+
+	for _, command := range []string{
+		cmdCheck, cmdFreshen, cmdList, cmdExport, cmdFetch,
+	} {
+		t.Run(command, func(t *testing.T) {
+			t.Parallel()
+
+			fs := afero.NewMemMapFs()
+			generateTestDir(t, fs)
+
+			opts := testOpts([]string{testApp, command, testDir, "/second"}, fs)
+			assert.Equal(t, 1, runCLI(opts))
+			assert.Contains(t, testStderr(t, opts),
+				`unexpected argument "/second"`)
+		})
+	}
+}
+
+// TestGenerateReadsPathAfterDoubleDash runs gen -- -x where -x is a
+// directory. -x must be read as the path to list, not as an option.
+//
+//nolint:paralleltest // changes the process-global working directory
+func TestGenerateReadsPathAfterDoubleDash(t *testing.T) {
+	chdirTemp(t)
+
+	fs := afero.NewOsFs()
+	require.NoError(t, fs.Mkdir("-x", 0o755))
+	writeTestFile(t, fs, filepath.Join("-x", testFileTxt), "content")
+
+	opts := testOpts([]string{testApp, cmdGenerate, "-q", "--", "-x"}, fs)
+	require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
+
+	files := manifestFiles(t, fs, filepath.Join("-x", defaultManifestName))
+	require.Len(t, files, 1)
+	assert.Equal(t, testFileTxt, files[0].GetPath())
 }
 
 func TestCheckDetectsManifestCorruption(t *testing.T) {

internal/cli/mfer.go

@@ -48,6 +48,10 @@ func manifestTempPath(out string) string {
 // errUnknownCommand indicates an unrecognized command argument.
 var errUnknownCommand = errors.New("unknown command")
 
+// errUnexpectedArgument indicates a second argument to a command that takes
+// one.
+var errUnexpectedArgument = errors.New("unexpected argument")
+
 // CLIApp is the main CLI application container. It holds configuration,
 // I/O streams, and filesystem abstraction to enable testing and flexibility.
 //
@@ -154,13 +158,14 @@ func commonFlags() []cli.Flag {
 	}
 }
 
-// stopOnFirstArg returns the StopOnNthArg setting every command uses: flags
-// are read only before the command's first argument, and everything after it
-// is an argument, as with urfave/cli v2. So `mfer gen d -v` names a path "-v".
-func stopOnFirstArg() *int {
-	n := 1
+// refuseExtraArgs is the ArgValidator of the commands that take one
+// argument: it refuses a second one, naming it.
+func refuseExtraArgs(_ context.Context, cmd *cli.Command) error {
+	if cmd.Args().Len() > 1 {
+		return fmt.Errorf("%w %q", errUnexpectedArgument, cmd.Args().Get(1))
+	}
 
-	return &n
+	return nil
 }
 
 // requireSignatureFlag returns the --require-signature flag taken by the
@@ -186,11 +191,10 @@ func includePermissionsFlag() *cli.BoolFlag {
 
 func (mfa *CLIApp) generateCommand() *cli.Command {
 	return &cli.Command{
-		Name:         cmdGenerate,
-		Aliases:      []string{"gen"},
-		Usage:        "Generate manifest file",
-		ArgsUsage:    "[path ...]",
-		StopOnNthArg: stopOnFirstArg(),
+		Name:      cmdGenerate,
+		Aliases:   []string{"gen"},
+		Usage:     "Generate manifest file",
+		ArgsUsage: "[path ...]",
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			mfa.setVerbosity(cmd)
 			mfa.printBanner()
@@ -252,7 +256,7 @@ func (mfa *CLIApp) checkCommand() *cli.Command {
 		Name:         cmdCheck,
 		Usage:        "Validate files using manifest file",
 		ArgsUsage:    manifestArgsUsage,
-		StopOnNthArg: stopOnFirstArg(),
+		ArgValidator: refuseExtraArgs,
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			mfa.setVerbosity(cmd)
 			mfa.printBanner()
@@ -286,7 +290,7 @@ func (mfa *CLIApp) freshenCommand() *cli.Command {
 		Name:         cmdFreshen,
 		Usage:        "Update manifest with changed, new, and removed files",
 		ArgsUsage:    manifestArgsUsage,
-		StopOnNthArg: stopOnFirstArg(),
+		ArgValidator: refuseExtraArgs,
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			mfa.setVerbosity(cmd)
 			mfa.printBanner()
@@ -337,7 +341,7 @@ func (mfa *CLIApp) exportCommand() *cli.Command {
 		Name:         cmdExport,
 		Usage:        "Export manifest contents as JSON",
 		ArgsUsage:    "[manifest file or URL]",
-		StopOnNthArg: stopOnFirstArg(),
+		ArgValidator: refuseExtraArgs,
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			mfa.setVerbosity(cmd)
 
@@ -348,9 +352,8 @@ func (mfa *CLIApp) exportCommand() *cli.Command {
 
 func (mfa *CLIApp) versionCommand() *cli.Command {
 	return &cli.Command{
-		Name:         cmdVersion,
-		Usage:        "Show version",
-		StopOnNthArg: stopOnFirstArg(),
+		Name:  cmdVersion,
+		Usage: "Show version",
 		Action: func(context.Context, *cli.Command) error {
 			mfa.printVersion()
 
@@ -365,7 +368,7 @@ func (mfa *CLIApp) listCommand() *cli.Command {
 		Aliases:      []string{"ls"},
 		Usage:        "List files in manifest",
 		ArgsUsage:    manifestArgsUsage,
-		StopOnNthArg: stopOnFirstArg(),
+		ArgValidator: refuseExtraArgs,
 		Action:       mfa.listManifestOperation,
 		Flags: []cli.Flag{
 			&cli.BoolFlag{
@@ -386,7 +389,7 @@ func (mfa *CLIApp) fetchCommand() *cli.Command {
 		Name:         cmdFetch,
 		Usage:        "fetch manifest and referenced files",
 		ArgsUsage:    "URL",
-		StopOnNthArg: stopOnFirstArg(),
+		ArgValidator: refuseExtraArgs,
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			mfa.setVerbosity(cmd)
 			mfa.printBanner()
@@ -445,9 +448,8 @@ func (mfa *CLIApp) run(args []string) {
 		Writer:                mfa.Stdout,
 		// v3 writes its "Incorrect Usage" line to ErrWriter; v2 wrote it to
 		// stdout, before the help, so it stays on stdout.
-		ErrWriter:    mfa.Stdout,
-		Flags:        commonFlags(),
-		StopOnNthArg: stopOnFirstArg(),
+		ErrWriter: mfa.Stdout,
+		Flags:     commonFlags(),
 		Action: func(_ context.Context, cmd *cli.Command) error {
 			if cmd.Args().Len() > 0 {
 				return fmt.Errorf("%w %q", errUnknownCommand, cmd.Args().First())

mfer/manifest_test.go

@@ -0,0 +1,30 @@
+//nolint:testpackage // white-box tests exercise unexported internals
+package mfer
+
+import (
+	"encoding/hex"
+	"testing"
+	"uuid"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestSignatureString checks the canonical string for the UUID and sha256
+// of the example in the Signature Scheme section of docs/FORMAT.md, which
+// shows the same string.
+func TestSignatureString(t *testing.T) {
+	t.Parallel()
+
+	manifestUUID := uuid.MustParse("b0fb6918-6298-443b-ada2-1b162f4dc0b3")
+	sha256Hash, err := hex.DecodeString(
+		"e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855")
+	require.NoError(t, err)
+
+	m := &manifest{pbOuter: &MFFileOuter{Uuid: manifestUUID[:], Sha256: sha256Hash}}
+
+	got, err := m.signatureString()
+	require.NoError(t, err)
+	assert.Equal(t, "ZNAVSRFG-b0fb69186298443bada21b162f4dc0b3-"+
+		"1220e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", got)
+}

mfer/mf.pb.go

@@ -223,11 +223,14 @@ type MFFileOuter struct {
 	// uuid must match the uuid in the inner message
 	Uuid         []byte `protobuf:"bytes,105,opt,name=uuid,proto3" json:"uuid,omitempty"`
 	InnerMessage []byte `protobuf:"bytes,199,opt,name=innerMessage,proto3" json:"innerMessage,omitempty"`
-	//detached signature, ascii or binary
+	// one OpenPGP signature, armored or binary, over the canonical string
+	// ZNAVSRFG-<uuid>-<sha256 as a multihash>, with the uuid as 32
+	// lower-case hex digits and no hyphens, and the multihash in lower-case
+	// hex (see docs/FORMAT.md). A signature is never stored anywhere else.
 	Signature []byte `protobuf:"bytes,201,opt,name=signature,proto3,oneof" json:"signature,omitempty"`
-	//full GPG key id
+	// fingerprint of the signing primary key, in upper-case hex
 	Signer []byte `protobuf:"bytes,202,opt,name=signer,proto3,oneof" json:"signer,omitempty"`
-	//full GPG signing public key, ascii or binary
+	// OpenPGP public key of the signer, one primary key, armored or binary
 	SigningPubKey []byte `protobuf:"bytes,203,opt,name=signingPubKey,proto3,oneof" json:"signingPubKey,omitempty"`
 	unknownFields protoimpl.UnknownFields
 	sizeCache     protoimpl.SizeCache

mfer/mf.proto

@@ -32,15 +32,17 @@ message MFFileOuter {
     bytes uuid = 105;
 
     bytes innerMessage = 199;
-    // 2xx for optional manifest root attributes
-    // think we might use gosignify instead of gpg:
-    // github.com/frankbraun/gosignify
 
-    //detached signature, ascii or binary
+    // 2xx for optional manifest root attributes
+
+    // one OpenPGP signature, armored or binary, over the canonical string
+    // ZNAVSRFG-<uuid>-<sha256 as a multihash>, with the uuid as 32
+    // lower-case hex digits and no hyphens, and the multihash in lower-case
+    // hex (see docs/FORMAT.md). A signature is never stored anywhere else.
     optional bytes signature = 201;
-    //full GPG key id
+    // fingerprint of the signing primary key, in upper-case hex
     optional bytes signer = 202;
-    //full GPG signing public key, ascii or binary
+    // OpenPGP public key of the signer, one primary key, armored or binary
     optional bytes signingPubKey = 203;
 }
 

mfer/mf.proto.sha256

@@ -1 +1 @@
-3d4dcb0b2f4640dd3ae6bb48b833e9f26ae9771e2d3e88bd646e7f1724dda654  mf.proto
+4f8ea5276a8e66590e71ed154a9176b0f68ffa9ab69d8de5aff0e0d9b731164d  mf.proto