Refuse an empty --require-signature or --sign-key (closes #205)

check, fetch, gen and freshen took --require-signature or --sign-key
given an empty value, on the command line or through
MFER_REQUIRE_SIGNATURE or MFER_SIGN_KEY, as the flag not given, so an
unset shell variable skipped the signature check or the signing. Each
now asks whether the flag was given, as gen's --output does. An empty
fingerprint is refused as one of the wrong length, by fetch before it
requests anything; an empty --sign-key is refused, naming the flag,
before any file is read.

Model: opus-5-5

internal/cli/check.go

@@ -329,10 +329,10 @@ func (mfa *CLIApp) checkManifestOperation(
 		return fmt.Errorf("load manifest: %w", err)
 	}
 
-	// Check signature requirement
+	// Check signature requirement. An empty fingerprint is checked too,
-	requiredSigner := cmd.String(flagRequireSignature)
+	// and refused as one of the wrong length.
-	if requiredSigner != "" {
+	if cmd.IsSet(flagRequireSignature) {
-		err = verifyRequiredSigner(chk, requiredSigner)
+		err = verifyRequiredSigner(chk, cmd.String(flagRequireSignature))
 		if err != nil {
 			return err
 		}

internal/cli/fetch.go

@@ -429,6 +429,14 @@ func (mfa *CLIApp) fetchManifestOperation(
 func (mfa *CLIApp) fetchManifest(
 	ctx context.Context, cmd *cli.Command, client retryingClient, manifestURL string,
 ) ([]byte, []*mfer.MFFilePath, error) {
+	// An empty fingerprint is refused before anything is requested, with
+	// the message check gives for it; any other is checked against the
+	// manifest once it is downloaded.
+	requiredSigner := cmd.String(flagRequireSignature)
+	if cmd.IsSet(flagRequireSignature) && requiredSigner == "" {
+		return nil, nil, fmt.Errorf("%w, got 0", errInvalidFingerprint)
+	}
+
 	log.Infof("fetching manifest from %s", manifestURL)
 
 	// Read the whole manifest before parsing it, so that a connection
@@ -460,7 +468,6 @@ func (mfa *CLIApp) fetchManifest(
 		return nil, nil, fmt.Errorf("parse manifest: %w", err)
 	}
 
-	requiredSigner := cmd.String(flagRequireSignature)
 	if requiredSigner != "" {
 		err = verifyFetchedSigner(manifestData, requiredSigner)
 		if err != nil {

internal/cli/freshen.go

@@ -351,7 +351,7 @@ func writeFreshenedManifest(
 }
 
 // newFreshenBuilder constructs the manifest builder configured from CLI
-// flags.
+// flags. A --sign-key given with an empty value is refused.
 func (mfa *CLIApp) newFreshenBuilder(cmd *cli.Command) (*mfer.Builder, error) {
 	builder := mfer.NewBuilder()
 	if cmd.Bool("include-timestamps") {
@@ -359,7 +359,12 @@ func (mfa *CLIApp) newFreshenBuilder(cmd *cli.Command) (*mfer.Builder, error) {
 	}
 
 	// Set up signing options if sign-key is provided
-	if signKey := cmd.String("sign-key"); signKey != "" {
+	if cmd.IsSet("sign-key") {
+		signKey := cmd.String("sign-key")
+		if signKey == "" {
+			return nil, errEmptySignKey
+		}
+
 		signing, err := mfa.signingOptions(signKey)
 		if err != nil {
 			return nil, err

internal/cli/gen.go

@@ -29,6 +29,8 @@ var (
 		"already exists (use --force to overwrite)")
 	// errEmptyOutput indicates --output given with an empty value.
 	errEmptyOutput = errors.New("--output must not be empty")
+	// errEmptySignKey indicates --sign-key given with an empty value.
+	errEmptySignKey = errors.New("--sign-key must not be empty")
 )
 
 // reportEnumProgress renders enumeration progress until the channel
@@ -121,7 +123,8 @@ func (mfa *CLIApp) outputPath(cmd *cli.Command) (string, error) {
 }
 
 // buildScannerOptions constructs scanner options from the CLI flags and
-// the path the manifest is written to.
+// the path the manifest is written to. A --sign-key given with an empty
+// value is refused.
 func (mfa *CLIApp) buildScannerOptions(
 	cmd *cli.Command, output string,
 ) (*mfer.ScannerOptions, error) {
@@ -144,7 +147,12 @@ func (mfa *CLIApp) buildScannerOptions(
 	}
 
 	// Set up signing options if sign-key is provided
-	if signKey := cmd.String("sign-key"); signKey != "" {
+	if cmd.IsSet("sign-key") {
+		signKey := cmd.String("sign-key")
+		if signKey == "" {
+			return nil, errEmptySignKey
+		}
+
 		signing, err := mfa.signingOptions(signKey)
 		if err != nil {
 			return nil, err

internal/cli/signing_test.go

@@ -4,8 +4,12 @@ package cli
 import (
 	"bufio"
 	"io"
+	"net/http"
+	"net/http/httptest"
 	"path/filepath"
+	"slices"
 	"strings"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -207,3 +211,105 @@ func TestGenAsksForPassphraseOnTerminal(t *testing.T) {
 	}, fs)
 	require.Equal(t, 0, runCLI(check), testStderr(t, check))
 }
+
+// assertEmptySignKeyRefused runs gen and freshen, each with flags, on a
+// directory holding a file and an unsigned manifest. Each must fail,
+// naming --sign-key, and leave the manifest as it was.
+func assertEmptySignKeyRefused(t *testing.T, flags ...string) {
+	t.Helper()
+
+	manifest := manifestOf(t, map[string][]byte{testFileTxt: []byte("listed")})
+
+	for command, rest := range map[string][]string{
+		cmdGenerate: {"-q", "-f", "-o", testMF, testDir},
+		cmdFreshen:  {"-q", testMF},
+	} {
+		fs := afero.NewMemMapFs()
+		require.NoError(t, fs.MkdirAll(testDir, 0o755))
+		writeTestFile(t, fs, testFile1, "hello")
+		require.NoError(t, afero.WriteFile(fs, testMF, manifest, 0o644))
+
+		opts := testOpts(slices.Concat([]string{testApp, command}, flags, rest), fs)
+		assert.Equal(t, 1, runCLI(opts), command)
+		assert.Contains(t, testStderr(t, opts), errEmptySignKey.Error(), command)
+
+		content, err := afero.ReadFile(fs, testMF)
+		require.NoError(t, err)
+		assert.Equal(t, manifest, content, command)
+	}
+}
+
+// TestGenAndFreshenRefuseEmptySignKey runs gen and freshen with --sign-key
+// given an empty value, as an unset shell variable gives it.
+func TestGenAndFreshenRefuseEmptySignKey(t *testing.T) {
+	t.Parallel()
+
+	assertEmptySignKeyRefused(t, testFlagSignKey, "")
+}
+
+// TestGenAndFreshenRefuseEmptySignKeyFromEnvironment runs gen and freshen
+// with MFER_SIGN_KEY set to an empty string.
+func TestGenAndFreshenRefuseEmptySignKeyFromEnvironment(t *testing.T) {
+	t.Setenv("MFER_SIGN_KEY", "")
+
+	assertEmptySignKeyRefused(t)
+}
+
+// assertEmptyFingerprintRefused runs check on an unsigned manifest, and
+// fetch of that manifest from a server, each with flags. Each must refuse
+// the empty fingerprint as malformed, and fetch must request nothing.
+func assertEmptyFingerprintRefused(t *testing.T, flags ...string) {
+	t.Helper()
+
+	const want = "invalid fingerprint: must be exactly 40 hex characters, got 0"
+
+	files := map[string][]byte{testFileTxt: []byte("listed")}
+	manifest := manifestOf(t, files)
+
+	fs := afero.NewMemMapFs()
+	require.NoError(t, fs.MkdirAll(testDir, 0o755))
+	writeTestFile(t, fs, filepath.Join(testDir, testFileTxt), "listed")
+	require.NoError(t, afero.WriteFile(fs, testMF, manifest, 0o644))
+
+	opts := testOpts(slices.Concat(
+		[]string{testApp, cmdCheck, "-q"}, flags, []string{testMF},
+	), fs)
+	assert.Equal(t, 1, runCLI(opts), cmdCheck)
+	assert.Contains(t, testStderr(t, opts), want, cmdCheck)
+
+	var requests atomic.Int32
+
+	tree := fetchTestHandler(manifest, files)
+
+	server := httptest.NewServer(
+		http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			requests.Add(1)
+			tree.ServeHTTP(w, r)
+		}))
+	defer server.Close()
+
+	opts = testOpts(slices.Concat(
+		[]string{testApp, cmdFetch, "-q", "--" + flagDest, t.TempDir()},
+		flags, []string{server.URL},
+	), afero.NewOsFs())
+	assert.Equal(t, 1, runCLI(opts), cmdFetch)
+	assert.Contains(t, testStderr(t, opts), want, cmdFetch)
+	assert.Zero(t, requests.Load(), "fetch made a request")
+}
+
+// TestCheckAndFetchRefuseEmptyRequireSignature runs check and fetch with
+// --require-signature given an empty value, as an unset shell variable
+// gives it.
+func TestCheckAndFetchRefuseEmptyRequireSignature(t *testing.T) {
+	t.Parallel()
+
+	assertEmptyFingerprintRefused(t, "--"+flagRequireSignature, "")
+}
+
+// TestCheckAndFetchRefuseEmptyRequireSignatureFromEnvironment runs check
+// and fetch with MFER_REQUIRE_SIGNATURE set to an empty string.
+func TestCheckAndFetchRefuseEmptyRequireSignatureFromEnvironment(t *testing.T) {
+	t.Setenv("MFER_REQUIRE_SIGNATURE", "")
+
+	assertEmptyFingerprintRefused(t)
+}