Read a path or URL named help or h as an argument (closes #198)

On gen, check, freshen, fetch, list and export, a first argument of
`help` or `h` ran urfave/cli's built-in help command, so
`check help --require-signature FPR` printed help and passed. Those
commands now set urfave/cli's HideHelpCommand, which leaves out their
help command; the root keeps its own, so `mfer help` and
`mfer help check` still work. `check DIR --help` failed with
`No help topic for 'DIR'`; their CommandNotFound now prints the
command's own help, as `check --help` does.

Model: opus-5-5

docs/FORMAT.md

@@ -174,16 +174,9 @@ 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.
 
-A signature is good only if the key that made it was valid at the signature's
-creation time. The primary key is judged by the newest self-signature on its
-primary user ID, and a subkey by its newest binding signature. By the
-signature's creation time the primary key and that self-signature must have been
-made and neither may have expired, and when a subkey made the signature the same
-holds of the subkey and its binding signature. A signature made before that
-self-signature or binding signature is not good, even if an older one made the
-key valid at that time. A key that has expired since does not make the signature
-bad, because a manifest outlives its key. A signature past its own expiry time
-is not good, whether or not the key has expired too.
+A signing key that has expired does not make the signature bad, because a
+manifest outlives its key. A signature past its own expiry time is not good,
+whether or not the key has expired too.
 
 ## Deterministic Serialization
 

internal/cli/entry_test.go

@@ -36,6 +36,8 @@ const (
 	testFlagNoExtra = "--no-extra-files"
 	testFlagVersion = "--version"
 	testFlagVerbose = "--verbose"
+	testFlagHelp    = "--help"
+	testCmdHelp     = "help"
 )
 
 var errSimulatedWrite = errors.New("simulated write failure")
@@ -321,7 +323,7 @@ func TestHelpCommand(t *testing.T) {
 	t.Parallel()
 
 	fs := afero.NewMemMapFs()
-	opts := testOpts([]string{testApp, "--help"}, fs)
+	opts := testOpts([]string{testApp, testFlagHelp}, fs)
 
 	exitCode := runCLI(opts)
 
@@ -1807,6 +1809,87 @@ func TestGenerateReadsPathAfterDoubleDash(t *testing.T) {
 	assert.Equal(t, testFileTxt, files[0].GetPath())
 }
 
+// TestPathNamedHelp runs gen, then check with --require-signature, on a
+// directory named help and on one named h, the two names of urfave/cli's
+// help command. gen must write the manifest into the directory, and check
+// must refuse it as unsigned.
+//
+//nolint:paralleltest // changes the process-global working directory
+func TestPathNamedHelp(t *testing.T) {
+	for _, name := range []string{testCmdHelp, "h"} {
+		t.Run(name, func(t *testing.T) {
+			chdirTemp(t)
+
+			fs := afero.NewOsFs()
+			require.NoError(t, fs.Mkdir(name, 0o755))
+			writeTestFile(t, fs, filepath.Join(name, testFileTxt), "content")
+
+			opts := testOpts([]string{testApp, cmdGenerate, "-q", name}, fs)
+			require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
+
+			files := manifestFiles(t, fs, filepath.Join(name, defaultManifestName))
+			require.Len(t, files, 1)
+
+			opts = testOpts([]string{
+				testApp, cmdCheck, "-q", name, "--" + flagRequireSignature, msgFpA,
+			}, fs)
+			assert.Equal(t, 1, runCLI(opts))
+			assert.Contains(t, testStderr(t, opts), "manifest is not signed")
+		})
+	}
+}
+
+// TestCommandsReadHelpAsArgument runs each command that takes a path or URL
+// on help and on h where nothing by that name exists. Each must fail on the
+// missing path or bad URL instead of printing its help.
+func TestCommandsReadHelpAsArgument(t *testing.T) {
+	t.Parallel()
+
+	for _, command := range []string{
+		cmdGenerate, cmdCheck, cmdFreshen, cmdList, cmdExport, cmdFetch,
+	} {
+		for _, name := range []string{testCmdHelp, "h"} {
+			t.Run(command+" "+name, func(t *testing.T) {
+				t.Parallel()
+
+				opts := testOpts([]string{testApp, command, name}, afero.NewMemMapFs())
+				assert.Equal(t, 1, runCLI(opts))
+				assert.NotContains(t, testStdout(t, opts), "USAGE:")
+			})
+		}
+	}
+}
+
+// TestHelpBesideArgument runs each command that takes a path or URL with
+// --help or -h before and after a directory, and runs help on that command.
+// Each must print what the command's own --help prints.
+func TestHelpBesideArgument(t *testing.T) {
+	t.Parallel()
+
+	for _, command := range []string{
+		cmdGenerate, cmdCheck, cmdFreshen, cmdList, cmdExport, cmdFetch,
+	} {
+		want := testOpts([]string{testApp, command, testFlagHelp}, afero.NewMemMapFs())
+		require.Equal(t, 0, runCLI(want))
+		require.Contains(t, testStdout(t, want), testApp+" "+command)
+
+		for _, args := range [][]string{
+			{command, testDir, testFlagHelp},
+			{command, testFlagHelp, testDir},
+			{command, testDir, "-h"},
+			{testCmdHelp, command},
+		} {
+			t.Run(strings.Join(args, " "), func(t *testing.T) {
+				t.Parallel()
+
+				opts := testOpts(slices.Concat([]string{testApp}, args), afero.NewMemMapFs())
+				assert.Equal(t, 0, runCLI(opts), testStderr(t, opts))
+				assert.Equal(t, testStdout(t, want), testStdout(t, opts))
+			})
+		}
+	}
+}
+
 func TestCheckDetectsManifestCorruption(t *testing.T) {
 	t.Parallel()
 

internal/cli/mfer.go

@@ -168,6 +168,17 @@ func refuseExtraArgs(_ context.Context, cmd *cli.Command) error {
 	return nil
 }
 
+// showCommandHelp is the CommandNotFound of the commands that take a path or
+// URL. Those commands set HideHelpCommand, so that a path or URL named help or
+// h is an argument rather than urfave/cli's help command; each sets it itself,
+// because on the root it would also remove `mfer help`. With --help,
+// urfave/cli looks the command's first argument up as a help topic, so
+// `check DIR --help` asks for a topic DIR and finds none; this prints the
+// command's own help instead, as `check --help` does.
+func showCommandHelp(ctx context.Context, cmd *cli.Command, _ string) {
+	_ = cli.ShowCommandHelp(ctx, cmd.Root(), cmd.Name)
+}
+
 // requireSignatureFlag returns the --require-signature flag taken by the
 // check and fetch subcommands.
 func requireSignatureFlag() *cli.StringFlag {
@@ -191,10 +202,12 @@ func includePermissionsFlag() *cli.BoolFlag {
 
 func (mfa *CLIApp) generateCommand() *cli.Command {
 	return &cli.Command{
-		Name:      cmdGenerate,
-		Aliases:   []string{"gen"},
-		Usage:     "Generate manifest file",
-		ArgsUsage: "[path ...]",
+		Name:            cmdGenerate,
+		Aliases:         []string{"gen"},
+		Usage:           "Generate manifest file",
+		ArgsUsage:       "[path ...]",
+		HideHelpCommand: true,
+		CommandNotFound: showCommandHelp,
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			mfa.setVerbosity(cmd)
 			mfa.printBanner()
@@ -253,10 +266,12 @@ func (mfa *CLIApp) generateCommand() *cli.Command {
 
 func (mfa *CLIApp) checkCommand() *cli.Command {
 	return &cli.Command{
-		Name:         cmdCheck,
-		Usage:        "Validate files using manifest file",
-		ArgsUsage:    manifestArgsUsage,
-		ArgValidator: refuseExtraArgs,
+		Name:            cmdCheck,
+		Usage:           "Validate files using manifest file",
+		ArgsUsage:       manifestArgsUsage,
+		ArgValidator:    refuseExtraArgs,
+		HideHelpCommand: true,
+		CommandNotFound: showCommandHelp,
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			mfa.setVerbosity(cmd)
 			mfa.printBanner()
@@ -287,10 +302,12 @@ func (mfa *CLIApp) checkCommand() *cli.Command {
 
 func (mfa *CLIApp) freshenCommand() *cli.Command {
 	return &cli.Command{
-		Name:         cmdFreshen,
-		Usage:        "Update manifest with changed, new, and removed files",
-		ArgsUsage:    manifestArgsUsage,
-		ArgValidator: refuseExtraArgs,
+		Name:            cmdFreshen,
+		Usage:           "Update manifest with changed, new, and removed files",
+		ArgsUsage:       manifestArgsUsage,
+		ArgValidator:    refuseExtraArgs,
+		HideHelpCommand: true,
+		CommandNotFound: showCommandHelp,
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			mfa.setVerbosity(cmd)
 			mfa.printBanner()
@@ -338,10 +355,12 @@ func (mfa *CLIApp) freshenCommand() *cli.Command {
 
 func (mfa *CLIApp) exportCommand() *cli.Command {
 	return &cli.Command{
-		Name:         cmdExport,
-		Usage:        "Export manifest contents as JSON",
-		ArgsUsage:    "[manifest file or URL]",
-		ArgValidator: refuseExtraArgs,
+		Name:            cmdExport,
+		Usage:           "Export manifest contents as JSON",
+		ArgsUsage:       "[manifest file or URL]",
+		ArgValidator:    refuseExtraArgs,
+		HideHelpCommand: true,
+		CommandNotFound: showCommandHelp,
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			mfa.setVerbosity(cmd)
 
@@ -364,12 +383,14 @@ func (mfa *CLIApp) versionCommand() *cli.Command {
 
 func (mfa *CLIApp) listCommand() *cli.Command {
 	return &cli.Command{
-		Name:         cmdList,
-		Aliases:      []string{"ls"},
-		Usage:        "List files in manifest",
-		ArgsUsage:    manifestArgsUsage,
-		ArgValidator: refuseExtraArgs,
-		Action:       mfa.listManifestOperation,
+		Name:            cmdList,
+		Aliases:         []string{"ls"},
+		Usage:           "List files in manifest",
+		ArgsUsage:       manifestArgsUsage,
+		ArgValidator:    refuseExtraArgs,
+		HideHelpCommand: true,
+		CommandNotFound: showCommandHelp,
+		Action:          mfa.listManifestOperation,
 		Flags: []cli.Flag{
 			&cli.BoolFlag{
 				Name:    "long",
@@ -386,10 +407,12 @@ func (mfa *CLIApp) listCommand() *cli.Command {
 
 func (mfa *CLIApp) fetchCommand() *cli.Command {
 	return &cli.Command{
-		Name:         cmdFetch,
-		Usage:        "fetch manifest and referenced files",
-		ArgsUsage:    "URL",
-		ArgValidator: refuseExtraArgs,
+		Name:            cmdFetch,
+		Usage:           "fetch manifest and referenced files",
+		ArgsUsage:       "URL",
+		ArgValidator:    refuseExtraArgs,
+		HideHelpCommand: true,
+		CommandNotFound: showCommandHelp,
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			mfa.setVerbosity(cmd)
 			mfa.printBanner()

mfer/openpgp.go

@@ -57,9 +57,7 @@ var (
 		"signature must hold exactly one signature")
 	errNotOneArmoredBlock = errors.New(
 		"must be exactly one armored block and nothing else")
-	errMalformedArmor        = errors.New("armor is malformed")
-	errKeyNotValidWhenSigned = errors.New(
-		"signature was made when its key was not valid")
+	errMalformedArmor = errors.New("armor is malformed")
 )
 
 // SigningOptions holds the key a manifest is signed with.
@@ -247,9 +245,8 @@ func fingerprint(key *openpgp.Entity) string {
 
 // verifySignature checks that signature is one good, unexpired OpenPGP
 // signature over data, made by the one primary key in pubKey or one of its
-// subkeys while that key was valid, and returns that primary key's
-// fingerprint. The key may have expired since. signature and pubKey may
-// each be armored or binary.
+// subkeys, and returns that primary key's fingerprint. signature and pubKey
+// may each be armored or binary.
 func verifySignature(data, signature, pubKey []byte) (string, error) {
 	key, err := readOneKey(pubKey, "embedded public key block", true)
 	if err != nil {
@@ -284,25 +281,6 @@ func verifySignature(data, signature, pubKey []byte) (string, error) {
 		return "", fmt.Errorf("verify signature: %w", err)
 	}
 
-	// The library checks whether the key is valid at the time its config
-	// gives, which above is now. Verifying again as of the signature's
-	// creation time refuses a signature made before the key or subkey that
-	// made it was made, after it expired, or after its self-signature or
-	// binding signature expired. The library judges the primary key by the
-	// newest self-signature on its primary user ID and a subkey by its
-	// newest binding signature, so this also refuses a signature made
-	// before either of those, even if an older one was valid then.
-	signedAt := &packet.Config{
-		Time: func() time.Time { return sig.CreationTime },
-	}
-
-	_, _, err = openpgp.VerifyDetachedSignature(openpgp.EntityList{key},
-		bytes.NewReader(data), bytes.NewReader(sigData), signedAt)
-	if err != nil {
-		return "", fmt.Errorf("verify signature: %w: %w",
-			errKeyNotValidWhenSigned, err)
-	}
-
 	return fingerprint(key), nil
 }
 

mfer/openpgp_test.go

@@ -4,7 +4,6 @@ package mfer
 import (
 	"bytes"
 	"context"
-	"crypto"
 	"crypto/dsa" //nolint:staticcheck // SA1019: tests need a DSA key to refuse
 	"io"
 	"math/big"
@@ -420,9 +419,8 @@ func TestVerifySignature(t *testing.T) {
 
 // TestVerifySignatureKeyExpiredSince verifies two signatures made in 2020
 // by a key that expired a day after it was made. A signature with no
-// expiry time is still good, as is one made then by a signing subkey that
-// expired a day after it was made. One that expired an hour after it was
-// made is refused, although the key has expired too.
+// expiry time is still good. One that expired an hour after it was made is
+// refused, although the key has expired too.
 func TestVerifySignatureKeyExpiredSince(t *testing.T) {
 	t.Parallel()
 
@@ -450,132 +448,10 @@ func TestVerifySignatureKeyExpiredSince(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, fingerprint(key), signer)
 
-	withSubkey := newTestKey(t, &packet.Config{Time: config.Time})
-	require.NoError(t, withSubkey.AddSigningSubkey(&packet.Config{
-		Algorithm: packet.PubKeyAlgoEdDSA, Time: config.Time,
-		KeyLifetimeSecs: config.KeyLifetimeSecs,
-	}))
-
-	var subkeySig bytes.Buffer
-
-	require.NoError(t, openpgp.ArmoredDetachSign(&subkeySig, withSubkey,
-		bytes.NewReader(data), config))
-
-	subkeyPubKey, err := armoredPublicKey(withSubkey)
-	require.NoError(t, err)
-
-	_, err = verifySignature(data, subkeySig.Bytes(), subkeyPubKey)
-	require.NoError(t, err)
-
 	_, err = verifySignature(data, expiringSig.Bytes(), pubKey)
 	require.ErrorIs(t, err, pgperrors.ErrSignatureExpired)
 }
 
-// signDated returns a detached signature over data by signer, dated when.
-// openpgp.DetachSign signs only with a key that is valid at the time its
-// config gives, so this makes the signature packet without that check.
-func signDated(t *testing.T, signer *packet.PrivateKey, data []byte,
-	when time.Time,
-) []byte {
-	t.Helper()
-
-	sig := &packet.Signature{
-		SigType:      packet.SigTypeBinary,
-		PubKeyAlgo:   signer.PubKeyAlgo,
-		Hash:         crypto.SHA256,
-		CreationTime: when,
-		IssuerKeyId:  &signer.KeyId,
-	}
-
-	h, err := sig.PrepareSign(nil)
-	require.NoError(t, err)
-
-	_, err = h.Write(data)
-	require.NoError(t, err)
-
-	require.NoError(t, sig.Sign(h, signer, nil))
-
-	var buf bytes.Buffer
-
-	require.NoError(t, sig.Serialize(&buf))
-
-	return buf.Bytes()
-}
-
-// TestVerifySignatureRefusesKeyNotValidWhenSigned verifies signatures
-// dated when the key or subkey that made them was not valid: after the key
-// expired; after the key's self-signature, or the signing subkey's binding
-// signature, expired, the key or subkey having expired since as well; and
-// before the key was made, also by a key made in 2100. Each must be
-// refused.
-func TestVerifySignatureRefusesKeyNotValidWhenSigned(t *testing.T) {
-	t.Parallel()
-
-	made := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)
-	twoDaysLater := made.Add(48 * time.Hour)
-	day := uint32((24 * time.Hour).Seconds())
-	clock := func(when time.Time) func() time.Time {
-		return func() time.Time { return when }
-	}
-
-	expiredAfterADay := newTestKey(t, &packet.Config{
-		Time: clock(made), KeyLifetimeSecs: day,
-	})
-	selfSignatureExpiredAfterADay := newTestKey(t, &packet.Config{
-		Time: clock(made), KeyLifetimeSecs: 30 * day, SigLifetimeSecs: day,
-	})
-
-	withSubkey := newTestKey(t, &packet.Config{Time: clock(made)})
-	require.NoError(t, withSubkey.AddSigningSubkey(&packet.Config{
-		Algorithm: packet.PubKeyAlgoEdDSA, Time: clock(made),
-		KeyLifetimeSecs: 30 * day, SigLifetimeSecs: day,
-	}))
-	subkey := withSubkey.Subkeys[len(withSubkey.Subkeys)-1].PrivateKey
-
-	madeADayLater := newTestKey(t, &packet.Config{
-		Time: clock(made.Add(24 * time.Hour)),
-	})
-	madeIn2100 := newTestKey(t, &packet.Config{
-		Time: clock(time.Date(2100, 1, 1, 0, 0, 0, 0, time.UTC)),
-	})
-
-	for name, test := range map[string]struct {
-		key    *openpgp.Entity
-		signer *packet.PrivateKey
-		when   time.Time
-	}{
-		"after the key expired": {
-			expiredAfterADay, expiredAfterADay.PrivateKey, twoDaysLater,
-		},
-		"after the self-signature expired": {
-			selfSignatureExpiredAfterADay,
-			selfSignatureExpiredAfterADay.PrivateKey, twoDaysLater,
-		},
-		"after the binding signature expired": {
-			withSubkey, subkey, twoDaysLater,
-		},
-		"before the key was made": {
-			madeADayLater, madeADayLater.PrivateKey, made,
-		},
-		"before a key made in 2100": {
-			madeIn2100, madeIn2100.PrivateKey, made,
-		},
-	} {
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-
-			data := []byte("signed in 2020")
-
-			pubKey, err := armoredPublicKey(test.key)
-			require.NoError(t, err)
-
-			_, err = verifySignature(data,
-				signDated(t, test.signer, data, test.when), pubKey)
-			require.ErrorIs(t, err, errKeyNotValidWhenSigned)
-		})
-	}
-}
-
 func TestManifestSignatureVerification(t *testing.T) {
 	t.Parallel()