Sign or timestamp an unchanged tree's manifest in freshen (closes #190)

freshen skipped the write whenever no file was added, changed or
removed, so --sign-key and --include-timestamps on an unchanged tree
did nothing. It now skips only when the manifest would come out the
same: nothing in the tree changed, the manifest is signed by the
--sign-key key (unsigned without one), and it has a createdAt
timestamp exactly when --include-timestamps asks for one. A file whose
recorded mode is not what --include-permissions asks for already
counts as changed.

Deviation: the mfer package gains SecretKeyFingerprint and a loaded
manifest's CreatedAt, which the comparison needs.

Model: opus-5-5

internal/cli/freshen.go

@@ -365,29 +365,39 @@ func writeFreshenedManifest(
 }
 
 // newFreshenBuilder constructs the manifest builder configured from CLI
-// flags. A --sign-key given with an empty value is refused.
-func (mfa *CLIApp) newFreshenBuilder(cmd *cli.Command) (*mfer.Builder, error) {
+// flags, and returns the fingerprint of the --sign-key key, or "" when
+// none is given. A --sign-key given with an empty value is refused.
+func (mfa *CLIApp) newFreshenBuilder(
+	cmd *cli.Command,
+) (*mfer.Builder, string, error) {
 	builder := mfer.NewBuilder()
 	if cmd.Bool("include-timestamps") {
 		builder.SetIncludeTimestamps(true)
 	}
 
+	var signKeyFingerprint string
+
 	// Set up signing options if sign-key is provided
 	if cmd.IsSet("sign-key") {
 		signKey := cmd.String("sign-key")
 		if signKey == "" {
-			return nil, errEmptySignKey
+			return nil, "", errEmptySignKey
 		}
 
 		signing, err := mfa.signingOptions(signKey)
 		if err != nil {
-			return nil, err
+			return nil, "", err
+		}
+
+		signKeyFingerprint, err = mfer.SecretKeyFingerprint(signing.SecretKey)
+		if err != nil {
+			return nil, "", fmt.Errorf("%s: %w", signKey, err)
 		}
 
 		builder.SetSigningOptions(signing)
 	}
 
-	return builder, nil
+	return builder, signKeyFingerprint, nil
 }
 
 // freshenScan runs the scan phase against the loaded manifest entries
@@ -499,12 +509,19 @@ func runFreshenHash(
 	return nil
 }
 
-// loadExistingEntries loads the manifest and indexes its file entries
+// existingManifest is what freshen uses of the manifest it rewrites.
+type existingManifest struct {
+	entriesByPath map[string]*mfer.MFFilePath
+	signer        string // fingerprint of the key it is signed with, "" if unsigned
+	hasCreatedAt  bool
+}
+
+// loadExistingManifest loads the manifest and indexes its file entries
 // by path. It refuses a signed manifest when no --sign-key is given, as
 // the rewritten manifest would be unsigned.
-func (mfa *CLIApp) loadExistingEntries(
+func (mfa *CLIApp) loadExistingManifest(
 	cmd *cli.Command, manifestPath string,
-) (map[string]*mfer.MFFilePath, error) {
+) (*existingManifest, error) {
 	log.Infof("loading manifest from %s", manifestPath)
 
 	// Load existing manifest
@@ -529,7 +546,11 @@ func (mfa *CLIApp) loadExistingEntries(
 		existingByPath[f.GetPath()] = f
 	}
 
-	return existingByPath, nil
+	return &existingManifest{
+		entriesByPath: existingByPath,
+		signer:        string(manifest.Signer()),
+		hasCreatedAt:  manifest.CreatedAt() != nil,
+	}, nil
 }
 
 func (mfa *CLIApp) freshenManifestOperation(
@@ -537,20 +558,18 @@ func (mfa *CLIApp) freshenManifestOperation(
 ) error {
 	log.Debug("freshenManifestOperation()")
 
-	showProgress := cmd.Bool("progress")
-
 	// Find manifest file
 	manifestPath, err := mfa.resolveFreshenManifestPath(cmd)
 	if err != nil {
 		return err
 	}
 
-	builder, err := mfa.newFreshenBuilder(cmd)
+	builder, signKeyFingerprint, err := mfa.newFreshenBuilder(cmd)
 	if err != nil {
 		return err
 	}
 
-	existingByPath, err := mfa.loadExistingEntries(cmd, manifestPath)
+	existing, err := mfa.loadExistingManifest(cmd, manifestPath)
 	if err != nil {
 		return err
 	}
@@ -562,7 +581,7 @@ func (mfa *CLIApp) freshenManifestOperation(
 
 	// Phase 1: Scan filesystem
 	scanner, removed, err := mfa.freshenScan(cmd, manifestPath, absBase,
-		existingByPath)
+		existing.entriesByPath)
 	if err != nil {
 		return err
 	}
@@ -579,7 +598,7 @@ func (mfa *CLIApp) freshenManifestOperation(
 	hasher := &freshenHasher{
 		fs:             mfa.Fs,
 		absBase:        absBase,
-		showProgress:   showProgress,
+		showProgress:   cmd.Bool("progress"),
 		totalHashBytes: totalHashBytes,
 		filesToHash:    filesToHash,
 		startHash:      time.Now(),
@@ -595,8 +614,14 @@ func (mfa *CLIApp) freshenManifestOperation(
 	log.Infof("freshen complete: %d unchanged, %d changed, %d added, %d removed",
 		scanner.unchanged, scanner.changed, scanner.added, removed)
 
-	// Skip writing if nothing changed
-	if scanner.changed == 0 && scanner.added == 0 && removed == 0 {
+	// Skip writing only when the manifest would come out the same: nothing
+	// in the tree changed, and the manifest already holds what --sign-key,
+	// --include-timestamps and --include-permissions ask for. The scan
+	// counts a file whose recorded mode is not what --include-permissions
+	// asks for as changed.
+	if scanner.changed == 0 && scanner.added == 0 && removed == 0 &&
+		existing.signer == signKeyFingerprint &&
+		existing.hasCreatedAt == cmd.Bool("include-timestamps") {
 		log.Infof("manifest unchanged, skipping write")
 
 		return nil

internal/cli/freshen_test.go

@@ -84,6 +84,23 @@ func manifestFiles(t *testing.T, fs afero.Fs, path string) []*mfer.MFFilePath {
 	return manifest.Files()
 }
 
+// manifestCreatedAt returns the createdAt timestamp of the manifest at
+// path, or nil if it has none.
+func manifestCreatedAt(t *testing.T, fs afero.Fs, path string) *mfer.Timestamp {
+	t.Helper()
+
+	runMu.Lock()
+	defer runMu.Unlock()
+
+	manifest, err := mfer.NewManifestFromFile(&mfer.ManifestFromFileOptions{
+		Path: path,
+		Fs:   fs,
+	})
+	require.NoError(t, err)
+
+	return manifest.CreatedAt()
+}
+
 // TestFreshenUnchanged freshens a tree that has not changed since gen
 // made its manifest: the manifest must list the same entries as before.
 func TestFreshenUnchanged(t *testing.T) {
@@ -141,6 +158,41 @@ func TestFreshenRecordsModeOnlyWhenAsked(t *testing.T) {
 		recordedModes())
 }
 
+// TestFreshenIncludeTimestampsOnUnchangedTree runs freshen
+// --include-timestamps on a tree that has not changed since gen made its
+// manifest, which has no createdAt timestamp. freshen must rewrite the
+// manifest with one. Run again with the flag it must leave the manifest as
+// it is; run without it, it must rewrite the manifest without one.
+func TestFreshenIncludeTimestampsOnUnchangedTree(t *testing.T) {
+	t.Parallel()
+
+	fs := afero.NewOsFs()
+	root, manifestPath := setupFreshenDir(t, fs, map[string]string{testFileTxt: "hello"})
+
+	freshenWithTimestamps := func() {
+		opts := testOpts([]string{
+			testApp, cmdFreshen, "-q", "--include-timestamps",
+			testFlagBase, root, manifestPath,
+		}, fs)
+		require.Equal(t, 0, runCLI(opts), "stderr: %s", testStderr(t, opts))
+	}
+
+	freshenWithTimestamps()
+	assert.NotNil(t, manifestCreatedAt(t, fs, manifestPath))
+
+	before, err := afero.ReadFile(fs, manifestPath)
+	require.NoError(t, err)
+
+	freshenWithTimestamps()
+
+	after, err := afero.ReadFile(fs, manifestPath)
+	require.NoError(t, err)
+	assert.Equal(t, before, after, "manifest that has a createdAt was rewritten")
+
+	runFreshen(t, fs, root, manifestPath)
+	assert.Nil(t, manifestCreatedAt(t, fs, manifestPath))
+}
+
 // assertManifestLists asserts that the manifest at manifestPath lists
 // exactly the files in want, each with the size and SHA-256 hash of its
 // content in want and the mtime of the file of that name under root.

internal/cli/signing_test.go

@@ -127,6 +127,53 @@ func TestFreshenSignedManifestNeedsSignKey(t *testing.T) {
 	assert.Len(t, manifestFiles(t, fs, manifestPath), 2)
 }
 
+// TestFreshenSignsUnchangedTree runs freshen with --sign-key on a tree that
+// has not changed since gen made its unsigned manifest. freshen must
+// rewrite the manifest signed by that key. Run again with the same key it
+// must leave the manifest as it is; with another key it must rewrite it
+// signed by that one.
+func TestFreshenSignsUnchangedTree(t *testing.T) {
+	t.Setenv(envSignKeyPassphrase, "")
+
+	fs := afero.NewOsFs()
+	_, manifestPath := setupFreshenDir(t, fs, map[string]string{testFileTxt: "hello"})
+
+	newKeyFile := func() (string, string) {
+		secretKey, fingerprint := testSecretKey(t, nil, nil)
+		keyFile := filepath.Join(t.TempDir(), "key.asc")
+		require.NoError(t, afero.WriteFile(fs, keyFile, secretKey, 0o600))
+
+		return keyFile, fingerprint
+	}
+
+	freshenAndCheckSignedBy := func(keyFile, fingerprint string) {
+		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))
+	}
+
+	keyFile, fingerprint := newKeyFile()
+	freshenAndCheckSignedBy(keyFile, fingerprint)
+
+	before, err := afero.ReadFile(fs, manifestPath)
+	require.NoError(t, err)
+
+	freshenAndCheckSignedBy(keyFile, fingerprint)
+
+	after, err := afero.ReadFile(fs, manifestPath)
+	require.NoError(t, err)
+	assert.Equal(t, before, after, "manifest already signed by the key was rewritten")
+
+	freshenAndCheckSignedBy(newKeyFile())
+}
+
 // 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.