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

@@ -5662,6 +5662,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
@@ -1172,6 +1176,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``
@@ -1646,6 +1651,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:

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

tests/pytests/unit/utils/test_secret.py

@@ -252,31 +252,60 @@ def test_serial_leaves_empty_string():
     assert secret.serial("") == ""
 
 
-def test_serial_leaves_non_string_scalars():
-    assert secret.serial(42) == 42
-    assert secret.serial(True) is True
+def test_serial_redacts_truthy_non_string_scalars():
+    # VCOPS-98852: serial() must redact ALL pillar value types, not just str,
+    # so it stays consistent with the repr path (_masked_repr), which already
+    # redacted truthy int/float/bool. Before this fix, serial(42) == 42 —
+    # a real leak through the exact function pillar.get() relies on.
+    assert secret.serial(42) == secret.REDACT_PLACEHOLDER
+    assert secret.serial(True) == secret.REDACT_PLACEHOLDER
+    assert secret.serial(3.14) == secret.REDACT_PLACEHOLDER
+
+
+def test_serial_leaves_falsy_non_string_scalars():
+    # Falsy/zero values and None are not treated as secrets (matches the
+    # pre-existing repr convention for _masked_repr).
+    assert secret.serial(0) == 0
+    assert secret.serial(False) is False
     assert secret.serial(None) is None
 
 
+def test_serial_redacts_bytes():
+    assert secret.serial(b"topsecret") == secret.REDACT_PLACEHOLDER.encode()
+
+
+def test_serial_leaves_empty_bytes():
+    assert secret.serial(b"") == b""
+
+
 def test_serial_redacts_masked_dict_strings():
     d = secret.MaskedDict({"password": "hunter2", "count": 3})
     result = secret.serial(d)
-    assert result == {"password": secret.REDACT_PLACEHOLDER, "count": 3}
+    assert result == {
+        "password": secret.REDACT_PLACEHOLDER,
+        "count": secret.REDACT_PLACEHOLDER,
+    }
 
 
 def test_serial_redacts_plain_dict_strings():
-    # serial is aggressive — also redacts strings in plain dicts
-    d = {"k": "v", "n": 1}
+    # serial is aggressive — also redacts strings (and other truthy scalars)
+    # in plain dicts
+    d = {"k": "v", "n": 1, "z": 0}
     result = secret.serial(d)
-    assert result == {"k": secret.REDACT_PLACEHOLDER, "n": 1}
+    assert result == {
+        "k": secret.REDACT_PLACEHOLDER,
+        "n": secret.REDACT_PLACEHOLDER,
+        "z": 0,
+    }
 
 
 def test_serial_redacts_nested():
-    d = secret.MaskedDict({"sub": {"s": "secret"}, "lst": ["a", 1]})
+    d = secret.MaskedDict({"sub": {"s": "secret"}, "lst": ["a", 1, 0]})
     result = secret.serial(d)
     assert result["sub"]["s"] == secret.REDACT_PLACEHOLDER
     assert result["lst"][0] == secret.REDACT_PLACEHOLDER
-    assert result["lst"][1] == 1
+    assert result["lst"][1] == secret.REDACT_PLACEHOLDER
+    assert result["lst"][2] == 0
 
 
 # ---------------------------------------------------------------------------
@@ -300,10 +329,11 @@ def test_mask_output_redacts_masked_dict():
 
 
 def test_mask_output_redacts_masked_list():
-    d = {"items": secret.MaskedList(["sensitive", 1])}
+    d = {"items": secret.MaskedList(["sensitive", 1, 0])}
     result = secret.mask_output(d)
     assert result["items"][0] == secret.REDACT_PLACEHOLDER
-    assert result["items"][1] == 1
+    assert result["items"][1] == secret.REDACT_PLACEHOLDER
+    assert result["items"][2] == 0
 
 
 def test_mask_output_nested_plain_dicts_not_redacted():