Add pillar_mask_output config option and fix full-value pillar masking

changelog/98852.added.md

@@ -0,0 +1 @@
+Added the ``pillar_mask_output`` master/minion config option. When set to ``False``, changes ``pillar.items``'s default (when the caller doesn't pass ``unmask``) to return unmasked pillar values, for sites relying on the pre-masking ``pillar.items`` behavior. Defaults to ``True`` (masked, matching existing behavior) and does not affect ``pillar.get``/``item``/``raw``/``ext``, ``no_log`` state output, or general CLI output, which keep redacting by default regardless of this setting.

changelog/98852.fixed.md

@@ -0,0 +1 @@
+Fixed pillar output masking (``salt.utils.secret.serial``) only redacting string values — truthy ``int``/``float``/``bool`` and non-empty ``bytes`` pillar values were returned unmasked through ``pillar.get`` and related functions even with masking enabled. Masking of these types is now consistent with how they were already redacted in ``repr``/``str`` output.

doc/ref/configuration/master.rst

@@ -5670,6 +5670,33 @@ Recursively merge lists by aggregating them instead of replacing them.
 
     pillar_merge_lists: False
 
+.. conf_master:: pillar_mask_output
+
+``pillar_mask_output``
+**********************
+
+.. versionadded:: 3008.3
+
+Default: ``True``
+
+Changes the *default* behavior of :py:func:`pillar.items
+<salt.modules.pillar.items>` when a caller doesn't explicitly pass
+``unmask``. When ``True`` (the default), ``pillar.items`` returns masked
+values (``**********``) by default, matching :py:func:`pillar.get
+<salt.modules.pillar.get>` and friends. Set to ``False`` to make
+``pillar.items`` default to returning real, unmasked values instead —
+useful for sites relying on the pre-masking ``pillar.items`` behavior.
+
+This option does **not** disable pillar masking elsewhere: ``pillar.get``,
+``pillar.item``, ``pillar.raw``, ``pillar.ext``, ``no_log`` state output,
+and the general CLI output safety net are unaffected and keep redacting by
+default regardless of this setting. Callers of ``pillar.items`` can always
+override the default explicitly with ``unmask=True``/``unmask=False``.
+
+.. code-block:: yaml
+
+    pillar_mask_output: True
+
 .. conf_master:: pillar_includes_override_sls
 
 ``pillar_includes_override_sls``

salt/config/__init__.py

@@ -698,6 +698,10 @@ def _gather_buffer_space():
         "pillar_source_merging_strategy": str,
         # Recursively merge lists by aggregating them instead of replacing them.
         "pillar_merge_lists": bool,
+        # When False, changes pillar.items()'s default (when the caller
+        # doesn't pass unmask=) to return unmasked pillar values. Does not
+        # affect pillar.get/item/raw/ext, no_log states, or general output.
+        "pillar_mask_output": bool,
         # If True, values from included pillar SLS targets will override
         "pillar_includes_override_sls": bool,
         # How to merge multiple top files from multiple salt environments
@@ -1173,6 +1177,7 @@ def _gather_buffer_space():
         "pillar_opts": False,
         "pillar_source_merging_strategy": "smart",
         "pillar_merge_lists": False,
+        "pillar_mask_output": True,
         "pillar_includes_override_sls": False,
         # ``pillar_cache``, ``pillar_cache_ttl``, ``pillar_cache_backend``,
         # ``gpg_cache``, ``gpg_cache_ttl`` and ``gpg_cache_backend``
@@ -1648,6 +1653,7 @@ def _gather_buffer_space():
         "pillar_safe_render_error": True,
         "pillar_source_merging_strategy": "smart",
         "pillar_merge_lists": False,
+        "pillar_mask_output": True,
         "pillar_includes_override_sls": False,
         "pillar_cache": False,
         "pillar_cache_ttl": 3600,

salt/modules/pillar.py

@@ -254,7 +254,10 @@ def items(
         :conf_minion:`pillarenv_from_saltenv`, and is otherwise ignored.
 
     unmask
-        If set to ``True``, the pillar data will be unmasked.
+        If set to ``True``, the pillar data will be unmasked. If not set, the
+        default is unmasked when either the current render context has
+        already disabled masking, or the :conf_minion:`pillar_mask_output`
+        config option is set to ``False``.
 
         .. versionadded:: 3008.0
 
@@ -297,7 +300,14 @@ def items(
     )
     ret = pillar.compile_pillar()
     if unmask is None:
-        unmask = not salt.utils.secret.mask_pillar.get()
+        # VCOPS-98852: pillar_mask_output only changes items()'s *default*
+        # when the caller didn't explicitly request masked/unmasked output —
+        # it does not disable masking elsewhere (pillar.get/item/raw/ext,
+        # no_log states, or the general output safety net keep their own
+        # existing behavior regardless of this option).
+        unmask = not salt.utils.secret.mask_pillar.get() or not __opts__.get(
+            "pillar_mask_output", True
+        )
     if unmask:
         return salt.utils.secret.expose(ret)
     else:

salt/utils/secret.py

@@ -62,18 +62,29 @@ def _mask_wrap(value):
     return value
 
 
+def _is_redactable_scalar(value) -> bool:
+    """True if value is a non-empty/truthy str, bytes, int, float, or bool leaf.
+
+    Shared by ``_masked_repr`` (display) and ``serial`` (actual output
+    boundary) so the two can't drift apart on which leaf values count as
+    sensitive — that drift is exactly what let non-string values leak
+    through ``serial()`` unmasked.
+    """
+    if isinstance(value, (str, bytes, int, float, bool)):
+        return bool(value)
+    return False
+
+
 def _masked_repr(value) -> str:
     """Build a redacted repr string for a MaskedDict or MaskedList."""
     if isinstance(value, dict):
         pairs = ", ".join(f"{k!r}: {_masked_repr(v)}" for k, v in value.items())
         return "{" + pairs + "}"
     if isinstance(value, list):
         return "[" + ", ".join(_masked_repr(v) for v in value) + "]"
-    if isinstance(value, str) and value:
-        return repr(REDACT_PLACEHOLDER)
-    if isinstance(value, bytes) and value:
+    if isinstance(value, bytes) and _is_redactable_scalar(value):
         return repr(REDACT_PLACEHOLDER.encode())
-    if isinstance(value, (int, float, bool)) and value:
+    if _is_redactable_scalar(value):
         return repr(REDACT_PLACEHOLDER)
     return repr(value)
 
@@ -244,7 +255,8 @@ def expose(value, _seen=None):
 
 
 def serial(value, _seen=None):
-    """Aggressively redact: replace ALL non-empty strings with REDACT_PLACEHOLDER.
+    """Aggressively redact: replace every non-empty/truthy scalar leaf value
+    (str, bytes, int, float, bool) with a redacted placeholder.
 
     Use at explicit pillar output boundaries (``pillar.get``, ``pillar.items``,
     ``pillar.item``, ``pillar.ext``) and inside ``no_log_mask``.
@@ -255,10 +267,12 @@ def serial(value, _seen=None):
     """
     if _seen is None:
         _seen = set()
-    if isinstance(value, str) and value:
+    if isinstance(value, bytes) and _is_redactable_scalar(value):
+        return REDACT_PLACEHOLDER.encode()
+    if _is_redactable_scalar(value):
         return REDACT_PLACEHOLDER
     if not isinstance(value, (dict, list)):
-        # int, float, bool, None, empty string, bytes — pass through
+        # int, float, bool, None, empty string, empty bytes — pass through
         return value
     vid = id(value)
     if vid in _seen:
@@ -312,10 +326,12 @@ def mask_output(value, _seen=None):
 
 
 def no_log_mask(state_ret):
-    """Replace ``comment`` and ``changes`` in a state return with redacted values.
+    """Replace ``name``, ``comment``, and ``changes`` in a state return with
+    redacted values.
 
     Called by ``salt/state.py`` when a state has ``no_log: True``.
     Mutates *state_ret* in place.
     """
+    state_ret["name"] = serial(state_ret["name"])
     state_ret["comment"] = serial(state_ret["comment"])
     state_ret["changes"] = serial(state_ret["changes"])

tests/pytests/integration/cli/test_salt_call.py

@@ -164,6 +164,7 @@ def test_local_sls_call_multiple_pillar_roots(salt_master, salt_call_cli):
             str(salt_master.pillar_tree.prod.paths[0]),
             "pillar.get",
             "some_dict",
+            unmask=True,
         )
         assert ret.returncode == 0
         assert "some_key1" in ret.data

tests/pytests/integration/modules/test_pillar.py

@@ -295,7 +295,7 @@ def test_pillar_refresh_pillar_get(salt_cli, salt_minion, key_pillar):
         key_pillar_instance.refresh_pillar()
 
         # The pillar can now be read from in-memory pillars
-        ret = salt_cli.run("pillar.get", key, minion_tgt=salt_minion.id)
+        ret = salt_cli.run("pillar.get", key, minion_tgt=salt_minion.id, unmask=True)
         assert ret.returncode == 0
         val = ret.data
         assert val is True, repr(val)
@@ -328,7 +328,7 @@ def test_pillar_refresh_pillar_item(salt_cli, salt_minion, key_pillar):
         key_pillar_instance.refresh_pillar()
 
         # The pillar can now be read from in-memory pillars
-        ret = salt_cli.run("pillar.item", key, minion_tgt=salt_minion.id)
+        ret = salt_cli.run("pillar.item", key, minion_tgt=salt_minion.id, unmask=True)
         assert ret.returncode == 0
         val = ret.data
         assert key in val
@@ -353,7 +353,7 @@ def test_pillar_refresh_pillar_items(salt_cli, salt_minion, key_pillar):
         # refresh_pillar event is fired.
         # Calling refresh_pillar to update in-memory pillars
         key_pillar_instance.refresh_pillar()
-        ret = salt_cli.run("pillar.items", minion_tgt=salt_minion.id)
+        ret = salt_cli.run("pillar.items", minion_tgt=salt_minion.id, unmask=True)
         assert ret.returncode == 0
         val = ret.data
         assert key in val
@@ -394,7 +394,7 @@ def test_pillar_refresh_pillar_ping(salt_cli, salt_minion, key_pillar):
         key_pillar_instance.refresh_pillar()
 
         # The pillar can now be read from in-memory pillars
-        ret = salt_cli.run("pillar.item", key, minion_tgt=salt_minion.id)
+        ret = salt_cli.run("pillar.item", key, minion_tgt=salt_minion.id, unmask=True)
         assert ret.returncode == 0
         val = ret.data
         assert key in val

tests/pytests/unit/modules/test_pillar.py

@@ -3,6 +3,7 @@
 import pytest
 
 import salt.modules.pillar as pillarmod
+import salt.utils.secret as secret
 from tests.support.mock import MagicMock, call, patch
 
 
@@ -135,23 +136,88 @@ def test_pillar_get_default_merge_regression_38558():
     """Test for pillar.get(key=..., default=..., merge=True)
     Do not update the ``default`` value when using ``merge=True``.
     See: https://github.com/saltstack/salt/issues/38558
+
+    ``res`` values below are masked (VCOPS-98852: pillar.get()'s default
+    output redacts truthy int/float/bool leaves too, not just strings) — use
+    ``unmask=True`` to assert against the real values. ``default`` is a plain
+    Python literal passed in by the caller, never itself redacted, so its
+    non-mutation check still compares real values.
     """
     with patch.dict(pillarmod.__pillar__, {"l1": {"l2": {"l3": 42}}}):
 
         res = pillarmod.get(key="l1")
-        assert {"l2": {"l3": 42}} == res
+        assert {"l2": {"l3": secret.REDACT_PLACEHOLDER}} == res
+        assert {"l2": {"l3": 42}} == pillarmod.get(key="l1", unmask=True)
 
         default = {"l2": {"l3": 43}}
 
         res = pillarmod.get(key="l1", default=default)
-        assert {"l2": {"l3": 42}} == res
+        assert {"l2": {"l3": secret.REDACT_PLACEHOLDER}} == res
         assert {"l2": {"l3": 43}} == default
 
         res = pillarmod.get(key="l1", default=default, merge=True)
-        assert {"l2": {"l3": 42}} == res
+        assert {"l2": {"l3": secret.REDACT_PLACEHOLDER}} == res
         assert {"l2": {"l3": 43}} == default
 
 
+def test_items_respects_pillar_mask_output_config_option():
+    """VCOPS-98852: ``pillar_mask_output`` only changes ``pillar.items``'s
+    *default* (when the caller doesn't pass ``unmask``) — per maintainer
+    feedback on saltstack/salt#69812, it must not disable masking wholesale.
+    """
+    compiled = {"pin": 1234}
+    pillar_obj = MagicMock()
+    pillar_obj.compile_pillar = MagicMock(return_value=compiled)
+    grains = MagicMock()
+    grains.value = MagicMock(return_value={})
+    with patch(
+        "salt.pillar.get_pillar", MagicMock(return_value=pillar_obj)
+    ), patch.object(pillarmod, "__grains__", grains, create=True):
+        with patch.dict(
+            pillarmod.__opts__,
+            {
+                "id": "minion",
+                "saltenv": "base",
+                "pillarenv": None,
+                "pillar_mask_output": False,
+            },
+        ):
+            assert pillarmod.items() == compiled
+
+        with patch.dict(
+            pillarmod.__opts__,
+            {
+                "id": "minion",
+                "saltenv": "base",
+                "pillarenv": None,
+                "pillar_mask_output": True,
+            },
+        ):
+            assert pillarmod.items() == {"pin": secret.REDACT_PLACEHOLDER}
+
+        # The caller's explicit unmask= always wins over the config default.
+        with patch.dict(
+            pillarmod.__opts__,
+            {
+                "id": "minion",
+                "saltenv": "base",
+                "pillarenv": None,
+                "pillar_mask_output": False,
+            },
+        ):
+            assert pillarmod.items(unmask=False) == {"pin": secret.REDACT_PLACEHOLDER}
+
+
+def test_pillar_get_ignores_pillar_mask_output_config_option():
+    """VCOPS-98852: ``pillar.get`` must keep masking by default regardless of
+    ``pillar_mask_output`` — that option only affects ``pillar.items``.
+    """
+    with patch.dict(pillarmod.__pillar__, {"pin": 1234}), patch.dict(
+        pillarmod.__opts__, {"pillar_mask_output": False}
+    ):
+        assert pillarmod.get(key="pin") == secret.REDACT_PLACEHOLDER
+
+
 def test_pillar_get_default_merge_regression_39062():
     """
     Confirm that we do not raise an exception if default is None and