comment_encryption: decouple comment encryption from value encryption

cmd/sops/encrypt.go

@@ -22,6 +22,7 @@ type encryptConfig struct {
 	EncryptedRegex          string
 	UnencryptedCommentRegex string
 	EncryptedCommentRegex   string
+	CommentEncryption       string
 	MACOnlyEncrypted        bool
 	KeyGroups               []sops.KeyGroup
 	GroupThreshold          int
@@ -85,6 +86,7 @@ func metadataFromEncryptionConfig(config encryptConfig) sops.Metadata {
 		EncryptedRegex:          config.EncryptedRegex,
 		UnencryptedCommentRegex: config.UnencryptedCommentRegex,
 		EncryptedCommentRegex:   config.EncryptedCommentRegex,
+		CommentEncryption:       config.CommentEncryption,
 		MACOnlyEncrypted:        config.MACOnlyEncrypted,
 		Version:                 version.Version,
 		ShamirThreshold:         config.GroupThreshold,

cmd/sops/main.go

@@ -1842,6 +1842,10 @@ func main() {
 			Name:  "encrypted-comment-regex",
 			Usage: "set the encrypted comment suffix. When specified, only keys that have comment matching the regex will be encrypted.",
 		},
+		cli.StringFlag{
+			Name:  "comment-encryption",
+			Usage: "control comment encryption independently of value encryption. One of \"plaintext\" or \"encrypted\". Cannot be used together with encrypted-comment-regex or unencrypted-comment-regex.",
+		},
 		cli.StringFlag{
 			Name:   "config",
 			Usage:  "path to sops' config file. If set, sops will not search for the config file recursively.",
@@ -2116,6 +2120,7 @@ func getEncryptConfig(c *cli.Context, fileName string, inputStore common.Store,
 	unencryptedRegex := c.String("unencrypted-regex")
 	encryptedCommentRegex := c.String("encrypted-comment-regex")
 	unencryptedCommentRegex := c.String("unencrypted-comment-regex")
+	commentEncryption := c.String("comment-encryption")
 	macOnlyEncrypted := c.GlobalBool("mac-only-encrypted")
 	var err error
 	if optionalConfig == nil {
@@ -2144,6 +2149,9 @@ func getEncryptConfig(c *cli.Context, fileName string, inputStore common.Store,
 		if unencryptedCommentRegex == "" {
 			unencryptedCommentRegex = optionalConfig.UnencryptedCommentRegex
 		}
+		if commentEncryption == "" {
+			commentEncryption = optionalConfig.CommentEncryption
+		}
 		if !macOnlyEncrypted {
 			macOnlyEncrypted = optionalConfig.MACOnlyEncrypted
 		}
@@ -2171,13 +2179,17 @@ func getEncryptConfig(c *cli.Context, fileName string, inputStore common.Store,
 		if encryptedCommentRegex != "" {
 			log.Warn(fmt.Sprintf("Using an encrypted comment regex does not make sense with the input store (the %s store never produces comments) and will be ignored.", inputStore.Name()))
 		}
+		if commentEncryption != "" {
+			log.Warn(fmt.Sprintf("Using comment-encryption does not make sense with the input store (the %s store never produces comments) and will be ignored.", inputStore.Name()))
+		}
 		// Do not warn about unencryptedCommentRegex and macOnlyEncrypted since they cannot have any effect.
 		unencryptedSuffix = ""
 		encryptedSuffix = ""
 		encryptedRegex = ""
 		unencryptedRegex = ""
 		encryptedCommentRegex = ""
 		unencryptedCommentRegex = ""
+		commentEncryption = ""
 		macOnlyEncrypted = false
 	}
 
@@ -2205,6 +2217,10 @@ func getEncryptConfig(c *cli.Context, fileName string, inputStore common.Store,
 		return encryptConfig{}, common.NewExitError("Error: cannot use more than one of encrypted_suffix, unencrypted_suffix, encrypted_regex, unencrypted_regex, encrypted_comment_regex, or unencrypted_comment_regex in the same file", codes.ErrorConflictingParameters)
 	}
 
+	if err := sops.ValidateCommentEncryption(commentEncryption, encryptedCommentRegex, unencryptedCommentRegex); err != nil {
+		return encryptConfig{}, common.NewExitError(fmt.Sprintf("Error: %s", err), codes.ErrorConflictingParameters)
+	}
+
 	// only supply the default UnencryptedSuffix when EncryptedSuffix, EncryptedRegex, and others are not provided
 	if cryptRuleCount == 0 && !isSingleValueStore {
 		unencryptedSuffix = sops.DefaultUnencryptedSuffix
@@ -2229,6 +2245,7 @@ func getEncryptConfig(c *cli.Context, fileName string, inputStore common.Store,
 		EncryptedRegex:          encryptedRegex,
 		UnencryptedCommentRegex: unencryptedCommentRegex,
 		EncryptedCommentRegex:   encryptedCommentRegex,
+		CommentEncryption:       commentEncryption,
 		MACOnlyEncrypted:        macOnlyEncrypted,
 		KeyGroups:               groups,
 		GroupThreshold:          threshold,

config/config.go

@@ -193,6 +193,7 @@ type creationRule struct {
 	EncryptedRegex          string      `yaml:"encrypted_regex"`
 	UnencryptedCommentRegex string      `yaml:"unencrypted_comment_regex"`
 	EncryptedCommentRegex   string      `yaml:"encrypted_comment_regex"`
+	CommentEncryption       string      `yaml:"comment_encryption"`
 	MACOnlyEncrypted        bool        `yaml:"mac_only_encrypted"`
 }
 
@@ -285,6 +286,7 @@ type Config struct {
 	EncryptedRegex          string
 	UnencryptedCommentRegex string
 	EncryptedCommentRegex   string
+	CommentEncryption       string
 	MACOnlyEncrypted        bool
 	Destination             publish.Destination
 	OmitExtensions          bool
@@ -489,6 +491,14 @@ func configFromRule(rule *creationRule, kmsEncryptionContext map[string]*string)
 		return nil, fmt.Errorf("error loading config: cannot use more than one of encrypted_suffix, unencrypted_suffix, encrypted_regex, unencrypted_regex, encrypted_comment_regex, or unencrypted_comment_regex for the same rule")
 	}
 
+	// comment_encryption is a separate, orthogonal setting: it does not participate in the
+	// six-way mutual exclusion above (it stays combinable with the four value-only
+	// selectors), but it directly conflicts with encrypted_comment_regex/
+	// unencrypted_comment_regex, which also decide comment encryption.
+	if err := sops.ValidateCommentEncryption(rule.CommentEncryption, rule.EncryptedCommentRegex, rule.UnencryptedCommentRegex); err != nil {
+		return nil, fmt.Errorf("error loading config: %s", err)
+	}
+
 	groups, err := getKeyGroupsFromCreationRule(rule, kmsEncryptionContext)
 	if err != nil {
 		return nil, err
@@ -503,6 +513,7 @@ func configFromRule(rule *creationRule, kmsEncryptionContext map[string]*string)
 		EncryptedRegex:          rule.EncryptedRegex,
 		UnencryptedCommentRegex: rule.UnencryptedCommentRegex,
 		EncryptedCommentRegex:   rule.EncryptedCommentRegex,
+		CommentEncryption:       rule.CommentEncryption,
 		MACOnlyEncrypted:        rule.MACOnlyEncrypted,
 	}, nil
 }

config/config_test.go

@@ -336,6 +336,48 @@ creation_rules:
     unencrypted_comment_regex: "sops:dec"
     `)
 
+var sampleConfigWithCommentEncryptionPlaintext = []byte(`
+creation_rules:
+  - path_regex: barbar*
+    kms: "1"
+    pgp: "2"
+    comment_encryption: plaintext
+    `)
+
+var sampleConfigWithCommentEncryptionEncrypted = []byte(`
+creation_rules:
+  - path_regex: barbar*
+    kms: "1"
+    pgp: "2"
+    comment_encryption: encrypted
+    `)
+
+var sampleConfigWithCommentEncryptionInvalidValue = []byte(`
+creation_rules:
+  - path_regex: barbar*
+    kms: "1"
+    pgp: "2"
+    comment_encryption: sometimes
+    `)
+
+var sampleConfigWithCommentEncryptionAndEncryptedCommentRegex = []byte(`
+creation_rules:
+  - path_regex: barbar*
+    kms: "1"
+    pgp: "2"
+    comment_encryption: plaintext
+    encrypted_comment_regex: "sops:enc"
+    `)
+
+var sampleConfigWithCommentEncryptionAndUnencryptedCommentRegex = []byte(`
+creation_rules:
+  - path_regex: barbar*
+    kms: "1"
+    pgp: "2"
+    comment_encryption: plaintext
+    unencrypted_comment_regex: "sops:dec"
+    `)
+
 var sampleConfigWithInvalidParameters = []byte(`
 creation_rules:
   - path_regex: foobar*
@@ -699,6 +741,91 @@ func TestLoadConfigFileWithEncryptedCommentRegex(t *testing.T) {
 	assert.Equal(t, "sops:enc", conf.EncryptedCommentRegex)
 }
 
+func TestLoadConfigFileWithCommentEncryptionPlaintext(t *testing.T) {
+	conf, err := parseCreationRuleForFile(parseConfigFile(sampleConfigWithCommentEncryptionPlaintext, t), "/conf/path", "barbar", nil)
+	assert.Equal(t, nil, err)
+	assert.Equal(t, "plaintext", conf.CommentEncryption)
+}
+
+func TestLoadConfigFileWithCommentEncryptionEncrypted(t *testing.T) {
+	conf, err := parseCreationRuleForFile(parseConfigFile(sampleConfigWithCommentEncryptionEncrypted, t), "/conf/path", "barbar", nil)
+	assert.Equal(t, nil, err)
+	assert.Equal(t, "encrypted", conf.CommentEncryption)
+}
+
+func TestLoadConfigFileWithCommentEncryptionInvalidValue(t *testing.T) {
+	_, err := parseCreationRuleForFile(parseConfigFile(sampleConfigWithCommentEncryptionInvalidValue, t), "/conf/path", "barbar", nil)
+	assert.NotNil(t, err)
+}
+
+// TestCommentEncryptionCompatibilityMatrix pins down the full compatibility surface:
+//
+//	Existing six selectors
+//	    │
+//	    └── remain mutually exclusive and unchanged
+//
+//	New comment_encryption
+//	    │
+//	    ├── compatible with:
+//	    │     encrypted_regex, unencrypted_regex, encrypted_suffix, unencrypted_suffix
+//	    │
+//	    └── incompatible with:
+//	          encrypted_comment_regex, unencrypted_comment_regex
+//
+// comment_encryption controls comments only, so it stays combinable with the four value-only
+// selectors, but conflicts with encrypted_comment_regex/unencrypted_comment_regex, which also
+// decide comment encryption.
+func TestCommentEncryptionCompatibilityMatrix(t *testing.T) {
+	compatible := map[string][]byte{
+		"unencrypted_regex": []byte(`
+creation_rules:
+  - path_regex: barbar*
+    kms: "1"
+    comment_encryption: plaintext
+    unencrypted_regex: "^dec:"
+`),
+		"encrypted_regex": []byte(`
+creation_rules:
+  - path_regex: barbar*
+    kms: "1"
+    comment_encryption: plaintext
+    encrypted_regex: "^enc:"
+`),
+		"unencrypted_suffix": []byte(`
+creation_rules:
+  - path_regex: barbar*
+    kms: "1"
+    comment_encryption: plaintext
+    unencrypted_suffix: "_unencrypted"
+`),
+		"encrypted_suffix": []byte(`
+creation_rules:
+  - path_regex: barbar*
+    kms: "1"
+    comment_encryption: plaintext
+    encrypted_suffix: "_enc"
+`),
+	}
+	for name, cfg := range compatible {
+		t.Run("compatible/"+name, func(t *testing.T) {
+			conf, err := parseCreationRuleForFile(parseConfigFile(cfg, t), "/conf/path", "barbar", nil)
+			assert.Nil(t, err, "comment_encryption + %s should be allowed", name)
+			assert.Equal(t, "plaintext", conf.CommentEncryption)
+		})
+	}
+
+	incompatible := map[string][]byte{
+		"encrypted_comment_regex":   sampleConfigWithCommentEncryptionAndEncryptedCommentRegex,
+		"unencrypted_comment_regex": sampleConfigWithCommentEncryptionAndUnencryptedCommentRegex,
+	}
+	for name, cfg := range incompatible {
+		t.Run("incompatible/"+name, func(t *testing.T) {
+			_, err := parseCreationRuleForFile(parseConfigFile(cfg, t), "/conf/path", "barbar", nil)
+			assert.NotNil(t, err, "comment_encryption + %s should be rejected", name)
+		})
+	}
+}
+
 func TestLoadConfigFileWithInvalidParameters(t *testing.T) {
 	_, err := parseCreationRuleForFile(parseConfigFile(sampleConfigWithInvalidParameters, t), "/conf/path", "foobar", nil)
 	assert.NotNil(t, err)

sops.go

@@ -63,6 +63,32 @@ import (
 // DefaultUnencryptedSuffix is the default suffix a TreeItem key has to end with for sops to leave its Value unencrypted
 const DefaultUnencryptedSuffix = "_unencrypted"
 
+// CommentEncryptionPlaintext is the Metadata.CommentEncryption value that forces every comment to
+// remain unencrypted, regardless of how associated values are encrypted.
+const CommentEncryptionPlaintext = "plaintext"
+
+// CommentEncryptionEncrypted is the Metadata.CommentEncryption value that forces every comment to
+// be encrypted, regardless of how associated values are encrypted.
+const CommentEncryptionEncrypted = "encrypted"
+
+// ValidateCommentEncryption checks a would-be CommentEncryption value against the rule shared by
+// every place that accepts one (the config file parser, the file-metadata loader, and the CLI
+// flag): it must be empty, CommentEncryptionPlaintext, or CommentEncryptionEncrypted, and it must
+// not be combined with an EncryptedCommentRegex/UnencryptedCommentRegex value, since both
+// mechanisms would then be deciding comment encryption at once. Returns nil when commentEncryption
+// is empty (the feature is opt-in) or valid and non-conflicting.
+func ValidateCommentEncryption(commentEncryption, encryptedCommentRegex, unencryptedCommentRegex string) error {
+	switch commentEncryption {
+	case "", CommentEncryptionPlaintext, CommentEncryptionEncrypted:
+	default:
+		return fmt.Errorf("invalid comment_encryption value %q, must be %q or %q", commentEncryption, CommentEncryptionPlaintext, CommentEncryptionEncrypted)
+	}
+	if commentEncryption != "" && (encryptedCommentRegex != "" || unencryptedCommentRegex != "") {
+		return fmt.Errorf("cannot use comment_encryption together with encrypted_comment_regex or unencrypted_comment_regex")
+	}
+	return nil
+}
+
 var DefaultDecryptionOrder = []string{age.KeyTypeIdentifier, pgp.KeyTypeIdentifier}
 
 type sopsError string
@@ -435,6 +461,12 @@ func (branch TreeBranch) walkBranch(in TreeBranch, path []string, commentsStack
 }
 
 func (tree Tree) shouldBeEncrypted(path []string, commentsStack [][]string, isComment bool) bool {
+	if isComment && tree.Metadata.CommentEncryption != "" {
+		// CommentEncryption is an orthogonal, opt-in override: when set, it alone decides
+		// whether a comment is encrypted, independently of the six value/comment selectors
+		// below. It never affects non-comment nodes, and it has no effect at all unless set.
+		return tree.Metadata.CommentEncryption == CommentEncryptionEncrypted
+	}
 	encrypted := true
 	if tree.Metadata.UnencryptedSuffix != "" {
 		for _, v := range path {
@@ -551,7 +583,7 @@ func (tree Tree) Encrypt(key []byte, cipher Cipher) (string, error) {
 				if err != nil {
 					return nil, fmt.Errorf("Could not encrypt value: %s", err)
 				}
-				if ok && tree.Metadata.UnencryptedCommentRegex != "" {
+				if ok && tree.Metadata.CommentEncryption == "" && tree.Metadata.UnencryptedCommentRegex != "" {
 					// If an encrypted comment matches tree.Metadata.UnencryptedCommentRegex, decryption will fail
 					// as the MAC does not match, and the commented value will not be decrypted.
 					// Note that cipher.Encrypt() returns a string, but we stored the result in an interface{}
@@ -672,13 +704,17 @@ func (tree *Tree) GenerateDataKeyWithKeyServices(svcs []keyservice.KeyServiceCli
 
 // Metadata holds information about a file encrypted by sops
 type Metadata struct {
-	LastModified              time.Time
-	UnencryptedSuffix         string
-	EncryptedSuffix           string
-	UnencryptedRegex          string
-	EncryptedRegex            string
-	UnencryptedCommentRegex   string
-	EncryptedCommentRegex     string
+	LastModified            time.Time
+	UnencryptedSuffix       string
+	EncryptedSuffix         string
+	UnencryptedRegex        string
+	EncryptedRegex          string
+	UnencryptedCommentRegex string
+	EncryptedCommentRegex   string
+	// CommentEncryption, when non-empty ("plaintext" or "encrypted"), decides comment
+	// encryption on its own, independently of the six selectors above. It is opt-in:
+	// leaving it empty preserves prior behavior exactly.
+	CommentEncryption         string
 	MessageAuthenticationCode string
 	MACOnlyEncrypted          bool
 	Version                   string