Skip to content

Make salt.loader.resource_modules deny-by-default (fixes #69881) - #69883

Open
dwoz wants to merge 1 commit into
saltstack:3008.xfrom
dwoz:dwoz/fix/69881-resource-loader-strict
Open

Make salt.loader.resource_modules deny-by-default (fixes #69881)#69883
dwoz wants to merge 1 commit into
saltstack:3008.xfrom
dwoz:dwoz/fix/69881-resource-loader-strict

Conversation

@dwoz

@dwoz dwoz commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

What's broken

salt.loader.resource_modules() layered per-type overlays over the full stock module set, so salt <resource-id> cmd.run … silently ran on the managing minion instead of failing with "Function 'X' is not supported for resource type 'Y'". Fixes #69881.

Fix

salt/loader/__init__.py: new _resource_type_module_dirs() helper accepts only each layer's resources/<rtype>/<ext_type>/ overlay. resource_modules() uses it — deny-by-default, __minion__ escape hatch unchanged.

Tests

  • Unit: test_per_resource_overrides.py, test_caller_resources.py — 14 passing.
  • Integration: test_resource_loader_strict.py — 6 passing (stock-module rejection, override reachability, managing-minion grains untouched).

Merge-forward

Base 3007.x; pipeline carries to 3008.x and master.

Fixes #69881

@dwoz
dwoz requested a review from a team as a code owner July 26, 2026 09:32
@dwoz dwoz added the test:full Run the full test suite label Jul 26, 2026
@dwoz dwoz added this to the Chlorine v3007.15 milestone Jul 26, 2026
@dwoz
dwoz force-pushed the dwoz/fix/69881-resource-loader-strict branch from f1baa55 to 93c4af4 Compare July 27, 2026 01:43
twangboy
twangboy previously approved these changes Jul 27, 2026
@dwoz
dwoz changed the base branch from 3007.x to 3008.x July 28, 2026 02:47
@dwoz
dwoz dismissed twangboy’s stale review July 28, 2026 02:47

The base branch was changed.

resource_modules() built its LazyLoader over _module_dirs(), which returns
the full stock module set (~100+ salt/modules/*) with per-type overlay
directories merely prepended.  That left every stock function reachable
in a resource-scoped loader.  Targeting a resource with a name like
cmd.run / grains.setval / file.remove / state.apply resolved through the
resource loader, ran the stock function in the managing minion process,
and returned the result attributed to the resource id — contradicting
the documented Resources safety contract (unsupported functions must
fail loudly; managing-minion access is explicit via __minion__).

Add a new private helper _resource_type_module_dirs() that walks the
same layer stack _module_dirs() walks (cli module_dirs, extension_modules,
entry-point packages, SALT_BASE_PATH) but only accepts each layer's
resources/<rtype>/<ext_type>/ overlay subdir — never the layer's plain
<ext_type>/ dir.  resource_modules() now uses this helper.

Result:

* salt <resource-id> <stock-fun> — "Function 'X' is not supported for
  resource type 'Y'." (the _thread_return guard fires because the name
  is absent from the loader).
* salt <resource-id> <per-type-fun> — reachable as before via the
  resources/<rtype>/modules/ overlay.
* Resource-context modules that intentionally want managing-minion
  behavior still call __minion__["x.y"] explicitly.

Tests:

* tests/pytests/unit/loader/test_per_resource_overrides.py
  - Flip test_no_override_falls_through_to_standard_state_module (which
    asserted stock state.sls was present) → test_no_override_hides_stock_modules
    asserting no stock cmd/state/grains/file/… leak through.  The prior
    assertion documented the buggy contract.
  - Add test_per_type_override_present_and_callable for the positive path.
* tests/pytests/unit/cli/test_caller_resources.py
  - salt-call -r grains.items / state.apply against a dummy resource
    now returns "not supported for resource type 'dummy'" rejections
    instead of stock function results.
* tests/pytests/integration/resources/test_resource_loader_strict.py (new)
  - End-to-end master+minion coverage: salt dummy-01 cmd.run /
    grains.setval / file.remove / sys.list_functions all return the
    per-type rejection; test.ping (a real per-type override) still runs;
    grains.setval leaves the managing minion's grains untouched.

Fixes saltstack#69881
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