[3006.x] Document gpg_decrypt_must_succeed opt-in and upcoming default flip

changelog/41846.deprecated.md

@@ -0,0 +1,9 @@
+Announced the upcoming default change of ``gpg_decrypt_must_succeed`` from
+``False`` to ``True``. On 3006.x the default remains ``False`` for LTS
+compatibility, but silent GPG decryption failures can produce
+ciphertext-as-plaintext in pillar data (issue #41846). Users are strongly
+encouraged to opt in now by setting ``gpg_decrypt_must_succeed: True`` in the
+minion or master config so that decryption failures raise
+``SaltRenderError`` instead of being silently ignored. The default flipped
+to ``True`` in the Potassium (3009.0) release and will flip in a future
+3006.x release; see the GPG renderer documentation for details.

salt/renderers/gpg.py

@@ -322,17 +322,35 @@
 Configuration
 *************
 
-The default behaviour of this renderer is to log a warning if a block could not
-be decrypted; in other words, it just returns the ciphertext rather than the
-encrypted secret.
+The default behaviour of this renderer on 3006.x is to log a warning if a block
+could not be decrypted; in other words, it just returns the ciphertext rather
+than the encrypted secret.
 
-This behaviour can be changed via the `gpg_decrypt_must_succeed` configuration
-option.  If set to `True`, any gpg block that cannot be decrypted raises a
-`SaltRenderError` exception, which registers an error in ``_errors`` during
-rendering.
+.. warning::
+
+    Silent decrypt failures can produce ciphertext-as-plaintext in pillar
+    data. Consumers such as ``file.managed``'s ``contents_pillar`` will then
+    write the raw GPG-armored ciphertext to disk in place of the decrypted
+    secret. See :issue:`41846`.
+
+    We recommend setting ``gpg_decrypt_must_succeed: True`` in the minion (or
+    master) config so that a decryption failure raises ``SaltRenderError``
+    and is registered in ``_errors`` during rendering, rather than being
+    silently ignored.
+
+    .. code-block:: yaml
 
-In the Chlorine release, the default behavior will be reversed and an error
-message will be added to ``_errors`` by default.
+        # /etc/salt/minion.d/gpg.conf
+        gpg_decrypt_must_succeed: True
+
+    The default was flipped to ``True`` in the Potassium (3009.0) release,
+    and will be flipped to ``True`` in a future 3006.x release. Users on
+    3006.x are encouraged to opt in now.
+
+This behaviour is controlled via the ``gpg_decrypt_must_succeed`` configuration
+option.  If set to ``True``, any gpg block that cannot be decrypted raises a
+``SaltRenderError`` exception, which registers an error in ``_errors`` during
+rendering.
 """
 
 import logging
@@ -488,11 +506,6 @@ def _decrypt_ciphertext(cipher):
                     decrypt_error,
                 )
             )
-        else:
-            salt.utils.versions.warn_until(
-                "Chlorine",
-                "After the Chlorine release of Salt, gpg_decrypt_must_succeed will default to True.",
-            )
         return cipher
     else:
         if __opts__.get("gpg_cache"):

tests/pytests/unit/renderers/test_gpg.py

@@ -83,6 +83,73 @@ def communicate(self, *args, **kwargs):
             assert "decrypt error" in multidecrypt_error.value.args[0]
 
 
+def test__decrypt_ciphertext_default_silent_on_3006_41846():
+    """
+    Regression test for #41846 (3006.x behavior).
+
+    On 3006.x the default value of ``gpg_decrypt_must_succeed`` is
+    ``False`` (kept for LTS backwards compatibility). A gpg decrypt
+    failure therefore returns the ciphertext unchanged and does NOT
+    raise. Users who want the fail-loud behavior must explicitly opt in
+    by setting ``gpg_decrypt_must_succeed: True`` in their minion or
+    master config; the ``test__decrypt_ciphertext`` fixture at the top of
+    this module already covers that path.
+
+    This test pins the silent-default contract on 3006.x so an
+    inadvertent flip is caught. The default is expected to flip to
+    ``True`` in a future 3006.x release.
+    """
+    import salt.config
+
+    opts = salt.config.DEFAULT_MINION_OPTS.copy()
+    assert opts["gpg_decrypt_must_succeed"] is False
+
+    key_dir = "/etc/salt/gpgkeys"
+    crypted = "-----BEGIN PGP MESSAGE-----!@#$%^&*()_+-----END PGP MESSAGE-----"
+
+    class GPGNotDecrypt:
+        def communicate(self, *args, **kwargs):
+            return [None, "decrypt error"]
+
+    with patch.dict(gpg.__opts__, opts), patch(
+        "salt.renderers.gpg._get_key_dir", MagicMock(return_value=key_dir)
+    ), patch("salt.utils.path.which", MagicMock()), patch(
+        "salt.renderers.gpg.Popen", MagicMock(return_value=GPGNotDecrypt())
+    ):
+        # Must not raise; must return the original ciphertext bytes.
+        result = gpg._decrypt_ciphertexts(crypted)
+        if isinstance(result, bytes):
+            result = result.decode()
+        assert crypted in result
+
+
+def test__decrypt_ciphertext_opt_in_raises_41846(minion_opts):
+    """
+    Regression test for #41846 opt-in path.
+
+    Users who explicitly set ``gpg_decrypt_must_succeed: True`` must see
+    a ``SaltRenderError`` on decrypt failure instead of a silent
+    return-ciphertext. This is the recommended opt-in for 3006.x.
+    """
+    key_dir = "/etc/salt/gpgkeys"
+    crypted = "-----BEGIN PGP MESSAGE-----!@#$%^&*()_+-----END PGP MESSAGE-----"
+
+    class GPGNotDecrypt:
+        def communicate(self, *args, **kwargs):
+            return [None, "decrypt error"]
+
+    opt_in = {"gpg_decrypt_must_succeed": True}
+    with patch.dict(gpg.__opts__, opt_in), patch(
+        "salt.renderers.gpg._get_key_dir", MagicMock(return_value=key_dir)
+    ), patch("salt.utils.path.which", MagicMock()), patch(
+        "salt.renderers.gpg.Popen", MagicMock(return_value=GPGNotDecrypt())
+    ):
+        with pytest.raises(SaltRenderError) as decrypt_error:
+            gpg._decrypt_ciphertexts(crypted)
+        assert decrypt_error.value.args[0].startswith("Could not decrypt cipher ")
+        assert crypted in decrypt_error.value.args[0]
+
+
 def test__decrypt_object():
     """
     test _decrypt_object