Skip to content

Add SafeDict/SafeList pillar wrapping with Pydantic secrets, output redaction, and no_log#68907

Open
Akm0d wants to merge 9 commits intosaltstack:masterfrom
Akm0d:fix/67367-mask-pillar
Open

Add SafeDict/SafeList pillar wrapping with Pydantic secrets, output redaction, and no_log#68907
Akm0d wants to merge 9 commits intosaltstack:masterfrom
Akm0d:fix/67367-mask-pillar

Conversation

@Akm0d
Copy link
Copy Markdown
Contributor

@Akm0d Akm0d commented Apr 7, 2026

What does this PR do?

This PR adds context-aware masking so sensitive pillar data is much harder to leak into state return payloads, minion logs, and event/log forwarding, while keeping explicit pillar APIs useful for debugging.

Highlights

  • pydantic>=2.4 in base requirements; SecretStr / SecretBytes for string/byte leaves.
  • salt.utils.safepillar: SafeDict / SafeList, wrap_pillar_tree / unwrap_pillar_tree, literal collection, substring redaction, no_log masking.
  • Compiled pillar is wrapped on minion-facing paths; automatic redaction replaces known pillar string literals in state returns with a fixed placeholder.
  • no_log: true on a state chunk (runtime keyword) masks that chunk’s comment and changes regardless of whether the secret text appears in pillar literals.
  • Thin (salt-ssh): Pydantic and its runtime deps are bundled in the thin tarball so salt-call from thin keeps working (fixes ModuleNotFoundError: pydantic / follow-on import errors in CI).
  • Direct pillar inspection: pillar.items and pillar.get return plain structures (unwrap_pillar_tree) so operators still see real values when they intentionally call those functions; in-memory pillar and accidental stringification stay protected.
  • Tests and pillar.rst updates.

Fixes: #67367 (mask pillar in state output / logs).


Previous Behavior

{'name': 'curl https://api.example.com',
 'comment': 'Request failed: invalid key sk-live-abc123',
 'changes': {'stdout': 'error: sk-live-abc123'},
 'result': False}

New Behavior

{'name': 'curl https://api.example.com',
 'comment': 'Request failed: invalid key **********',
 'changes': {'stdout': 'error: **********'},
 'result': False}

2) no_log: true on a state

Before: comment and changes are whatever the state module returned (may include secrets not present as pillar literals).

After: both are masked to the placeholder shape:

comment: **********
changes: {'**********': '**********'}

3) In-memory pillar vs pillar.items / pillar.get

  • Wrapped leaf: str(pillar['db']['password'])********** (safe if something logs or stringifies the object).
  • pillar.items-style unwrap{'db': {'password': 'plain-in-pillar'}} (plain values for intentional inspection).

Merge requirements

  • Docs (pillar.rst)
  • Changelog (per Salt guidelines)
  • Tests (unit + integration where applicable)
  • salt-ssh thin includes Pydantic stack (CI: test_thin_dir)

Reviewer notes

  • Custom code that assumes isinstance(x, str) for every pillar leaf may need to accept SecretStr or call .get_secret_value() after unwrap at boundaries.
  • Longest-secret-first literal replacement avoids leaking a shorter secret that is a substring of a longer one.

…edaction, and no_log

- Declare pydantic in base requirements and document behavior in the pillar tutorial.
- Introduce salt.utils.safepillar: SafeDict, SafeList, SecretStr/SecretBytes, unwrap for cache, literal collection, and redaction helpers.
- Wrap compiled pillar on minion-facing paths (remote/async pillar, cache read path, State pillar, gen_modules, pillar.items / pillar.ext).
- Redact known pillar string literals in per-state returns and in state.* job returns/logs on the minion.
- Add runtime no_log on state chunks (via STATE_RUNTIME_KEYWORDS) to mask comment / changes.
- Add unit tests for safe containers and integration tests for redaction / no_log (run under full salt-factories CI).
@Akm0d Akm0d requested a review from a team as a code owner April 7, 2026 22:50
@dwoz dwoz added the test:full Run the full test suite label Apr 8, 2026
Unwrap minion_blackout_whitelist before comparing to function_name so
SecretStr entries match str job names.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:full Run the full test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants