From 60dbcc03bc7f9a3f6e3dfd08ef5a89172bc060a3 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Wed, 14 Oct 2026 00:00:27 +0200 Subject: [PATCH] Refuse to freshen a signed manifest without --sign-key (closes #191) freshen built the new manifest from its flags alone, so a signed manifest freshened without --sign-key came out unsigned while the log said its signature verified. freshen now stops after loading such a manifest, before scanning, with an error naming the signer's fingerprint and --sign-key, and leaves the manifest as it was. With --sign-key it rewrites and signs as before. The library gains Signer() on the loaded manifest: the signing key's fingerprint, or nil when the manifest is unsigned, as Checker.Signer gives it. Model: opus-5-5 --- README.md | 3 +++ internal/cli/freshen.go | 15 ++++++++--- internal/cli/signing_test.go | 50 ++++++++++++++++++++++++++++++++++++ mfer/manifest.go | 11 ++++++++ mfer/openpgp_test.go | 19 ++++++++++++++ 5 files changed, 95 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index fdc96a7..b8482ee 100644 --- a/README.md +++ b/README.md @@ -300,6 +300,9 @@ are now tracked only in the [issues](https://git.eeqj.de/sneak/mfer/issues). - leaves out hidden files unless given `--include-dotfiles`, and symlinks unless given `--follow-symlinks`, which lists each symlink to a regular file under its own name with the contents of the file it points to + - refuses a signed manifest, leaving it as it was and naming the fingerprint + of the key that signed it, unless `--sign-key` or `MFER_SIGN_KEY` names a + key to sign the new one with - `mfer gen --sign-key key.asc` / `mfer freshen --sign-key key.asc` - signs the manifest with the OpenPGP secret key in `key.asc`, armored or binary, as `gpg --export-secret-keys` writes it; `MFER_SIGN_KEY` names the diff --git a/internal/cli/freshen.go b/internal/cli/freshen.go index cb37312..042eead 100644 --- a/internal/cli/freshen.go +++ b/internal/cli/freshen.go @@ -38,6 +38,10 @@ var ( errEntryMissingHash = errors.New("manifest entry has no hash") ) +// errSignKeyNeeded indicates a signed manifest given to freshen with no +// key to sign the rewritten one. +var errSignKeyNeeded = errors.New("give --sign-key to keep it signed") + // FreshenStatus contains progress information for the freshen operation. type FreshenStatus struct { Phase string // "scan" or "hash" @@ -481,9 +485,10 @@ func runFreshenHash( } // loadExistingEntries loads the manifest and indexes its file entries -// by path. +// by path. It refuses a signed manifest when no --sign-key is given, as +// the rewritten manifest would be unsigned. func (mfa *CLIApp) loadExistingEntries( - manifestPath string, + cmd *cli.Command, manifestPath string, ) (map[string]*mfer.MFFilePath, error) { log.Infof("loading manifest from %s", manifestPath) @@ -496,6 +501,10 @@ func (mfa *CLIApp) loadExistingEntries( return nil, fmt.Errorf("load manifest: %w", err) } + if signer := manifest.Signer(); signer != nil && cmd.String("sign-key") == "" { + return nil, fmt.Errorf("manifest is signed by %s: %w", signer, errSignKeyNeeded) + } + existingFiles := manifest.Files() log.Infof("manifest contains %d files", len(existingFiles)) @@ -526,7 +535,7 @@ func (mfa *CLIApp) freshenManifestOperation( return err } - existingByPath, err := mfa.loadExistingEntries(manifestPath) + existingByPath, err := mfa.loadExistingEntries(cmd, manifestPath) if err != nil { return err } diff --git a/internal/cli/signing_test.go b/internal/cli/signing_test.go index 77062ba..84ee654 100644 --- a/internal/cli/signing_test.go +++ b/internal/cli/signing_test.go @@ -73,6 +73,56 @@ func TestGenAndFreshenSignWithKeyFile(t *testing.T) { } } +// TestFreshenSignedManifestNeedsSignKey runs freshen on a signed manifest +// after a file is added. Without --sign-key it must fail, naming the +// signer and --sign-key, and leave the manifest as it was; with --sign-key +// it must rewrite the manifest, signed. +func TestFreshenSignedManifestNeedsSignKey(t *testing.T) { + t.Setenv(envSignKeyPassphrase, "") + + secretKey, fingerprint := testSecretKey(t, nil, nil) + + fs := afero.NewOsFs() + keyFile := filepath.Join(t.TempDir(), "key.asc") + root := t.TempDir() + manifestPath := filepath.Join(root, defaultManifestName) + + require.NoError(t, afero.WriteFile(fs, keyFile, secretKey, 0o600)) + writeTestFile(t, fs, filepath.Join(root, testFileTxt), "hello") + + opts := testOpts([]string{ + testApp, cmdGenerate, "-q", testFlagSignKey, keyFile, + "-o", manifestPath, root, + }, fs) + require.Equal(t, 0, runCLI(opts), testStderr(t, opts)) + + before, err := afero.ReadFile(fs, manifestPath) + require.NoError(t, err) + + writeTestFile(t, fs, filepath.Join(root, "added.txt"), "added") + + opts = testOpts([]string{testApp, cmdFreshen, "-q", manifestPath}, fs) + assert.Equal(t, 1, runCLI(opts)) + assert.Contains(t, testStderr(t, opts), fingerprint) + assert.Contains(t, testStderr(t, opts), testFlagSignKey) + + after, err := afero.ReadFile(fs, manifestPath) + require.NoError(t, err) + assert.Equal(t, before, after) + + opts = testOpts([]string{ + testApp, cmdFreshen, "-q", testFlagSignKey, keyFile, manifestPath, + }, fs) + require.Equal(t, 0, runCLI(opts), testStderr(t, opts)) + + opts = testOpts([]string{ + testApp, cmdCheck, "-q", + "--" + flagRequireSignature, fingerprint, manifestPath, + }, fs) + require.Equal(t, 0, runCLI(opts), testStderr(t, opts)) + assert.Len(t, manifestFiles(t, fs, manifestPath), 2) +} + // TestSignWithProtectedKeyNeedsPassphrase runs gen with a protected key, // with MFER_SIGN_KEY_PASSPHRASE empty and no terminal to ask on. gen must // fail, naming the variable, and write no manifest. diff --git a/mfer/manifest.go b/mfer/manifest.go index b172a12..88527a6 100644 --- a/mfer/manifest.go +++ b/mfer/manifest.go @@ -47,6 +47,17 @@ func (m *manifest) Files() []*MFFilePath { return m.pbInner.GetFiles() } +// 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 { + if len(m.pbOuter.GetSignature()) == 0 { + return nil + } + + return m.pbOuter.GetSigner() +} + // 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. diff --git a/mfer/openpgp_test.go b/mfer/openpgp_test.go index c6673f3..0a93c1d 100644 --- a/mfer/openpgp_test.go +++ b/mfer/openpgp_test.go @@ -176,6 +176,25 @@ func TestBuilderWithSigning(t *testing.T) { "-----BEGIN PGP PUBLIC KEY BLOCK-----") } +// TestManifestSigner checks that Signer gives the signing key's +// fingerprint for a signed manifest, and nil once the signature is taken +// out, although the signer field is still there. +func TestManifestSigner(t *testing.T) { + t.Parallel() + + key := newTestKey(t, nil) + data := signedTestManifest(t, &SigningOptions{SecretKey: armoredSecretKeys(t, key)}) + + signed, err := NewManifestFromReader(bytes.NewReader(data)) + require.NoError(t, err) + assert.Equal(t, fingerprint(key), string(signed.Signer())) + + unsigned, err := NewManifestFromReader(bytes.NewReader( + rewriteOuter(t, data, func(outer *MFFileOuter) { outer.Signature = nil }))) + require.NoError(t, err) + assert.Nil(t, unsigned.Signer()) +} + func TestScannerWithSigning(t *testing.T) { t.Parallel()