[3006.x] Add SALT_ONEDIR_HARDEN opt-in for daemon writable-state isolation (#70198) - #70208
Merged
dwoz merged 22 commits intoSep 9, 2026
Merged
Conversation
Introduce SALT_ONEDIR_HARDEN=1 as an opt-in packaging mode that
relocates each daemon's writable state under per-daemon
/var/lib/salt/<daemon>/{home,extras-<py>} directories so
/opt/saltstack/salt stays root:root 0755. On 3006.x the default is
UNSET (legacy chown -R salt /opt/saltstack/salt behavior preserved)
so existing LTS deployments continue to work without intervention.
The default flips to hardened on 3009.0 (saltstack#70199).
To opt in on 3006.x, set SALT_ONEDIR_HARDEN=1 in /etc/default/salt-setup
(DEB) or /etc/sysconfig/salt-minion-setup (RPM) before installing or
upgrading.
Runtime salt-pip and the onedir _salt_onedir_extras .pth hook honor
SALT_EXTRAS_DIR so packages installed via salt-pip continue to be
importable by the daemon after the packaging layer relocates the
extras tree. Postinst / posttrans scriptlets migrate any populated
legacy /opt/saltstack/salt/extras-<py>/ contents into the per-daemon
location once (when the operator opts in), idempotent no-op otherwise.
Sibling of saltstack#70199 (master) per the per-branch rollout in issue saltstack#70198.
Per-daemon fanout for the writable-dir existence/ownership checks (minion/master/syndic/api/cloud), belt-and-braces asserts for /opt/saltstack/salt files staying root-owned when hardening opt-in is selected, an explicit unset-vs-1 gate check that pins the 3006.x legacy default so an accidental default flip on LTS fails CI loud, and multi-role isolation verification that each daemon's /var/lib/salt/<daemon>/ is a distinct directory with no cross-daemon file visibility. Also includes a runtime contract test for salt-pip honoring SALT_EXTRAS_DIR. The _hardened_mode_selected() gate on 3006.x is opposite of master: here, SALT_ONEDIR_HARDEN unset skips the hardened-layout assertions (legacy is the default). Operators must explicitly export SALT_ONEDIR_HARDEN=1 for the hardened tests to run. Refs saltstack#70198.
New sibling directory tests/pytests/pkg/integration/onedir_harden_lifecycle/ covers what the session-scoped install_salt fixture can't on 3006.x: - SALT_ONEDIR_HARDEN unset (3006.x default): legacy chown layout, per-daemon /var/lib/salt/<daemon>/ dirs absent - SALT_ONEDIR_HARDEN=1 opt-in: hardened layout, /opt/saltstack/salt stays root-owned - Explicit SALT_HOME and SALT_EXTRAS_DIR overrides win over the HARDEN=1 opt-in (per-daemon dirs replaced by operator-chosen paths) - Upgrade migration: pre-populates /opt/saltstack/salt/extras-<py>/ with a marker under HARDEN unset, then reinstalls with HARDEN=1 and asserts marker moved into /var/lib/salt/<daemon>/extras-<py>/ - Idempotency: dpkg-reconfigure / dnf reinstall doesn't resurrect the legacy dir or duplicate the marker Mirrors the config_overrides/ destructive-fixture pattern (collection-skip on upgrade/downgrade/no-install passes, per-test apt/rpm purge, force-purge on postinst failure). 7 new tests. The "HARDEN=0 escape hatch" and "deprecation warning surfaces" tests from master are omitted because on 3006.x the default is already unset (no opt-out needed) and there is no deprecation warning (that lands with the default flip on 3009.0). Refs saltstack#70198.
twangboy
previously approved these changes
Sep 3, 2026
Six is bundled in the onedir at /opt/saltstack/salt/lib/python*/site-packages, so 'salt-pip install --no-deps six' returns 0 with 'Requirement already satisfied' and never writes to the SALT_EXTRAS_DIR --target override. The override dir is left empty and the test's post-install assertion fails. Switch to pep8 -- known-not-bundled per the existing test_pip.py::test_pip_install_extras which uses pep8 for exactly this reason. This makes the runtime SALT_EXTRAS_DIR contract actually observable end-to-end. Failed uniformly across every install/upgrade/ downgrade Test Package job on this PR.
Round 2's fix (246085c) quarantined sources.list.d 3rd-party feeds during the fixture-yield so mod_repo's internal apt-get update wouldn't trip a dead upstream mirror. But the teardown restored the quarantined entries FIRST and THEN called aptpkg.refresh_db() -- which re-hit the same 404 (apt.releases.hashicorp.com bullseye) and raised CommandExecutionError during teardown, making test_mod_repo report ERROR even though the test itself passed. Two changes: - Run the teardown refresh_db() while the quarantine is still in place, then restore. - Wrap it in try/except CommandExecutionError so first-party feed hiccups can't red the job either -- best-effort refresh, the test is done.
Downgrade jobs (--use-prev-version) run the test suite against the previously-released 3006.x version. That version predates the _resolve_extras_dir hook added to salt/scripts.py in this PR, so its salt-pip still hard-codes <relenv_root>/extras-<py> and ignores SALT_EXTRAS_DIR. The runtime contract this test verifies can't hold on the old code -- skip cleanly instead of masking as a real regression. Same install_salt.use_prev_version pattern already used by tests/pytests/pkg/integration/test_pip_urllib3_patch.py. Failing uniformly across every Linux Test Package downgrade job (AL2023, Debian 11-13, Photon 4/5, Rocky 8/9, Ubuntu 22.04/24.04).
test_upgrade_migration_moves_legacy_extras failed on Photon 4/5 install jobs while passing on every other RPM distro (Rocky 8/9, AL2023). The migration guard required [ -d "$SALT_EXTRAS_DIR" ] but that dir was only created inside the if [ $1 -gt 1 ]; else branch. When RPM reports $1 > 1 on what looks-like-an-install to us (e.g. a purge that leaves rpmdb metadata behind, which the fixture's _purge_salt_packages hits on Photon's tdnf shim), the install-time mkdir doesn't run, the migration guard fails on the -d check, and the legacy marker stays put. Fix: move install -d -o $SALT_USER -g $SALT_GROUP $SALT_EXTRAS_DIR inside the migration block itself, guarded on SALT_ONEDIR_HARDEN=1 + non-empty legacy source. Idempotent: if the dir already exists this is a no-op. Empty-target precondition still guards the mv so a re-run doesn't clobber existing per-daemon content. Applied to all 4 daemon posttrans blocks (cloud/master/syndic/api) which all had the same shape.
Two Photon-tdnf-specific bugs remained after round-4's migration-block fix: 1. test_upgrade_migration_moves_legacy_extras (Photon 4/5 install): The destructive-cycle fixture calls ``yum remove`` between installs. On Photon, ``yum`` is a tdnf shim that sometimes leaves rpmdb metadata behind for removed packages. The next install then reports ``$1 > 1`` in %posttrans scriptlets even though the fixture treats it as a fresh install. Round-4 patched the migration block to create ``$SALT_EXTRAS_DIR`` unconditionally, but the fresh-install ``chown -R salt /opt/saltstack/salt`` and per-daemon ``install -d`` still live in the ``if [ $1 -gt 1 ]; then :; else ... ; fi`` gate and never fire on Photon. Fix at the fixture: after ``yum remove``, run ``rpm -e --nodeps --allmatches --noscripts`` to force-drop the rpmdb entry so the next install sees ``$1 == 1``. 2. test_hardening_unset_matches_legacy_default (Photon 5 Arm64 upgrade): after an RPM upgrade on Photon, /opt/saltstack/salt gets reset to root ownership even though the previous package version had chowned it to salt. Rocky/AL2023/Debian/Ubuntu upgrades preserve the tree ownership; only Photon resets. The %posttrans ``$1 > 1`` gate then skips the corrective chown. Add a post-branch healer in %posttrans cloud/master/syndic/api/minion that re-chowns /opt/saltstack/salt back to $SALT_USER:$SALT_GROUP if it's currently root-owned AND SALT_ONEDIR_HARDEN != 1. Only fires when the tree is root-owned, so operator overrides to any non-root user are preserved. Idempotent no-op on distros that preserve ownership. Both fixes are additive; no existing branch semantics change. The healer is a corrective on top of the ``$1 > 1`` preserve-ownership policy, gated to only heal an accidental root-reset in the legacy layout.
The two-package npm install (``pm2@5.1.0`` + ``grunt@1.5.3`` from the public npm registry) occasionally exceeds pytest's default 90-second timeout under CI network jitter. Observed on Rocky Linux 9 functional zeromq split 3 in the run for PR saltstack#70208 while the same test passed green on the 3006.x nightly baseline the day before -- a flake, not a regression from this PR. The sibling ``test_removed_installed_cycle`` in the same file already carries ``@pytest.mark.timeout_unless_on_windows(120)`` for the exact same reason (single-package install of pm2). Give the two-package case 180s to leave headroom for the extra network round-trip. This hardens a legitimately-slow test against transient network jitter; it does not skip or paper over any real bug in either the NPM state module or this PR's onedir-harden changes (the test doesn't exercise either).
…on investigation Round-5's rpm -e --allmatches --noscripts fix in the destructive-cycle fixture unblocked Photon 5 Arm64 upgrade but Photon 4/5 install still fail test_upgrade_migration_moves_legacy_extras with the same fingerprint: legacy marker not migrated, no per-daemon marker. Rocky/AL2023/Debian/Ubuntu all pass. All conditions look correct on paper: - test_harden_on_produces_hardened_layout PASSES on Photon (proves the fresh-install ``install -d $SALT_EXTRAS_DIR`` branch runs). - test_explicit_salt_extras_dir_wins_over_harden_default PASSES on Photon (proves SALT_EXTRAS_DIR env-file plumbing works). - The migration block runs unconditionally after round-4's fix and requires only that /opt/saltstack/salt/extras-<py>/ exists with content -- the fixture places that AFTER purge and BEFORE the hardened install. So either tdnf's install wipes the fixture-created legacy extras dir before %posttrans runs, or the migration block's mv silently fails on Photon, or SALT_EXTRAS_DIR isn't set the way we think when the migration block executes. Instrument all 4 daemon %posttrans blocks (cloud/master/syndic/api) to write a diagnostic dump to /var/log/salt/posttrans-diag.log capturing $1, SALT_ONEDIR_HARDEN, SALT_USER/GROUP, SALT_HOME, SALT_EXTRAS_DIR, PY_VER, the sysconfig file contents, and dir listings of both /opt/saltstack/salt/ extras-<py>/ and SALT_EXTRAS_DIR itself. Attach the diagnostic file to the test failure assertion messages so it lands in the CI log. Once the Photon-specific bug is identified and fixed, revert the diagnostic block. Silent no-op on every distro; only writes if /var/log/salt exists (created by salt-common package).
Round-5's rpm -e --allmatches fix unblocked Photon 5 Arm64 upgrade but
Photon 4/5 install still failed test_upgrade_migration_moves_legacy_extras
with the same fingerprint: legacy marker at
/opt/saltstack/salt/extras-3.11/.onedir-harden-migration-marker never
moved into /var/lib/salt/<daemon>/extras-3.11/. Rocky/AL2023/Debian/
Ubuntu all pass.
Root cause: the fixture's design placed the marker between a full
package PURGE and a full package REINSTALL. On Photon's tdnf, the
purge removes /opt/saltstack/salt/ (which is packaged as a whole
tree by salt-common), so between purge and reinstall the fixture
must recreate /opt/saltstack/salt/extras-3.11/ and drop the marker
back in. tdnf's install phase then either wipes those untracked files
during package extraction, or the marker gets shadowed by the
/opt/saltstack/salt/ tree replay, so by the time %posttrans master's
migration block runs the ls -A check on /opt/saltstack/salt/
extras-3.11/ comes up empty and the migration is skipped. Non-Photon
RPM distros (Rocky/AL2023) preserve the untracked marker across the
extraction phase; Photon does not.
Fix: replace the purge+reinstall dance with an in-place reinstall
that keeps /opt/saltstack/salt/ intact between the two %posttrans
runs. Sequence:
1. Fresh install with SALT_ONEDIR_HARDEN unset (legacy default).
2. Drop marker into /opt/saltstack/salt/extras-<py>/ (survives
because we do not purge before the second run).
3. Write SALT_ONEDIR_HARDEN=1 into /etc/sysconfig/salt-minion-setup
(or /etc/default/salt-setup on DEB) so the next %posttrans /
postinst reads the opt-in.
4. ``rpm --reinstall <pkg paths>`` (or ``apt-get install --reinstall``
on DEB) reruns every daemon's %pre/%post/%posttrans while
leaving untracked files under /opt/saltstack/salt/ alone,
which is exactly the state the migration block needs to fire.
5. Yield the installer for the test to assert the marker moved.
The new flow also more faithfully mirrors what a real operator does
when upgrading legacy -> hardened on 3006.x: bump the sysconfig
knob and reinstall / upgrade the salt packages, no purge in the
middle. ``rpm --reinstall --nodigest --nosignature`` mirrors the
``--nogpgcheck`` tdnf install flag SaltPkgInstall already uses for
unsigned nightly ``3006.27+NNN.gSHA`` artifacts.
Round-6 added a diagnostic-only write of /var/log/salt/posttrans-diag.log from every %posttrans cloud/master/syndic/api scriptlet to trace why the Photon migration test was failing. The fixture rewrite in the same round (commit 516658f) fixed the migration test on Photon 4/5 install -- verified in round-7 CI logs: both migration tests PASSED on the affected jobs. The diagnostic is no longer needed. But keeping the diagnostic write regressed 8 previously-green pkg jobs (AL2023 install/upgrade, Rocky 8 Arm64 install/upgrade, Rocky 9 install/upgrade, Photon 4 upgrade, Photon 5 Arm64 upgrade). Two distinct failure modes, both rooted in the same file: 1. test_pkg_paths (install jobs) walks every file under /var/log/salt and asserts non-salt_user paths are root-owned. %posttrans minion runs ``chown -R $_MN_USER:$_MN_GROUP ... /var/log/salt`` (baseline behavior since 3006.0), which recursively chowned the new posttrans-diag.log to salt. Assertion failed at tests/pytests/pkg/integration/test_salt_user.py:317 with ``assert 'salt' == 'root'``, file_path=/var/log/salt/posttrans-diag.log. 2. test_salt_user_ownership_preserved_on_upgrade (upgrade jobs) inverts the check: after upgrade with salt user config, expects NO root-owned files under /var/log/salt. On Photon-5-arm the minion chown didn't reach the diagnostic file (racing scriptlet order?), so it stayed root-owned and this assertion tripped with ``Found root-owned files in /var/log/salt after running salt-call/salt-pip: /var/log/salt/posttrans-diag.log``. Both failure modes are 100% caused by the diagnostic write. Fixture rewrite is confirmed working; drop the diagnostic entirely. Reverts the file writes from commit 045b101 (in the spec) and the _dump_posttrans_diag helper + assertion-message plumbing from the lifecycle test. No other round-6 changes are touched -- the fixture rewrite in commit 516658f stays.
Debian 11 (bullseye) reached EOL on 2026-08-31 and the
debian-security bullseye-security InRelease signature has since
expired. The pkg-test session's autouse ``_system_up_to_date``
fixture asserts ``apt update`` returned 0, so every Debian 11
Test Package job (install / upgrade / downgrade, x86_64 and
arm64) now fast-fails at fixture setup with::
E: Release file for
http://deb.debian.org/debian-security/dists/bullseye-security/InRelease
is expired (invalid since 1d ...).
AssertionError: assert 100 == 0
Switch to ``apt-get`` with ``-o Acquire::Check-Valid-Until=false``
so the fixture can still refresh the package lists on an EOL
distro. Gpg signature verification is unaffected; only the
Valid-Until timestamp is ignored. Salt packages are still
``apt-mark hold``'d for the ``apt-get upgrade`` step so a
distro-wide upgrade cannot replace the onedir under test.
Local reproduction (debian:11 image):
before: apt-get update -> RC=100
after: apt-get update -o ...=false -> RC=0
Round 8's fix (e281a24) applied ``-o Acquire::Check-Valid-Until=false`` to every Debian version's ``apt-get update`` / ``apt-get upgrade`` in the pkg-test bootstrap fixture. Only Debian 11 (bullseye) actually needs the bypass -- it reached EOL on 2026-08-31 and its debian-security InRelease is no longer being refreshed. Debian 12 (bookworm) and Debian 13 (trixie) are still supported and their Valid-Until timestamp is a real security signal that a stale mirror or downgrade attack would fail; silencing it across all Debian versions widens the trust surface unnecessarily. Gate the bypass on ``grains["osmajorrelease"] == "11"`` so only bullseye skips the freshness check. Bookworm and trixie continue to fail loudly on a stale archive, as intended.
Round-8/9 fixed the pkg-test bootstrap fixture's ``apt-get update`` to bypass the expired debian-security bullseye-security InRelease on Debian 11 (bullseye reached EOL 2026-08-31). That patch only covered ``tests/pytests/pkg/`` -- the Test Salt functional and scenarios jobs run tests that shell out to ``apt-get update`` via other paths (most commonly ``salt.modules.aptpkg`` invoked from a state), and they still raise ``CommandExecutionError: E: Release file ... is expired`` on the same 4 Debian 11 x86_64 jobs (functional zeromq 1/2/3 + scenarios). Add a session-scoped autouse fixture in the top-level pytests conftest that drops ``/etc/apt/apt.conf.d/99-salt-tests-bullseye-eol`` disabling only ``Acquire::Check-Valid-Until`` for the session, scoped to bullseye by inspecting ``/etc/os-release``'s ``VERSION_CODENAME``. GPG signature verification stays enforced. Debian 12 (bookworm) and Debian 13 (trixie) are unaffected -- their Valid-Until is still a real security signal. No-op on non-Debian, on non-bullseye Debian, and when we lack write access to ``/etc/apt/apt.conf.d/`` (unprivileged runs).
The ``compat`` scenario test spawns ``ghcr.io/saltstack/salt-ci-containers/salt:{3002,3003,3004}``
containers and runs ``state.highstate`` with ``pkg.installed: figlet``. That
triggers ``apt update`` inside the container which now fails with expired
bullseye-security InRelease -- Debian 11 EOL'd 2026-08-31 and its security
mirror is no longer refreshed. These images are pinned/immutable old-release
containers so we can't just rebuild them.
Fix at the fixture layer: register an ``after_start`` callback on the
``SaltMinion`` container factory that ``docker exec``s a small snippet
rewriting ``/etc/apt/sources.list`` to point at ``snapshot.debian.org``
(bullseye snapshot pre-EOL) and dropping
``/etc/apt/apt.conf.d/99-salt-tests-bullseye-eol`` with
``Acquire::Check-Valid-Until "false";``. Fires before ``state.highstate``
is issued, so the minion's ``pkg.installed`` call finds a working apt.
Matches the pattern the salt-ci-containers ``testing:debian-11`` fresh-image
fix uses, and complements the host-level ``_bullseye_eol_apt_bypass`` session
fixture in ``tests/pytests/conftest.py`` (which does not reach inside spawned
containers). Best-effort: log and continue on non-Debian containers.
Locally verified end-to-end against ``salt:3002`` and ``salt:3004``:
``apt-get update`` succeeds and ``apt-get install figlet`` completes cleanly
after the snippet is applied.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Sibling of #70199 (master) for 3006.x. Per issue #70198's per-branch rollout,
SALT_ONEDIR_HARDENdefault = unset on 3006.x (legacy blanket-chown of/opt/saltstack/salt/preserved). Operators opt in explicitly viaSALT_ONEDIR_HARDEN=1in/etc/default/salt-setupor/etc/sysconfig/salt-<daemon>-setupto get the hardened layout early on LTS.When
SALT_ONEDIR_HARDEN=1:SALT_HOMEdefault →/var/lib/salt/<daemon>/homeSALT_EXTRAS_DIRdefault →/var/lib/salt/<daemon>/extras-<py>/opt/saltstack/salt/staysroot:root 0755.SALT_ONEDIR_HARDEN=1, populated legacy/opt/saltstack/salt/extras-<py>/migrates to/var/lib/salt/<daemon>/extras-<py>/.Existing
SALT_HOME/SALT_EXTRAS_DIRoverrides continue to work.Default flips to
SALT_ONEDIR_HARDEN=1on 3009.0 (master, #70199).Fixes #70198 for 3006.x.
Test plan
test:fulllabel applied).