You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
SECURITY: Escape filetree_create exports, keep its output inside output_path, and mark inventory plugin data unsafe - #340
This fixes three issues where text stored in the controller can be templated or written somewhere unintended on the machine that runs Ansible. Two were reported externally against filetree_create. The third, in the controller inventory plugin, was found while validating those reports.
Changes
filetree_create: export escaping
The export templates wrote controller strings into YAML without escaping. When filetree_read loaded the files, Jinja in a name or description was evaluated on the control node. A label named ops{{ 6 * 7 }} came back as ops42. A quote and a newline in a value could also add new keys to the exported entry.
New filter ctrliq.ascender.export_yaml writes each value as one line of flow-style YAML. Strings are double-quoted and tagged !unsafe.
All 22 templates in roles/filetree_create/templates/ and playbooks/flatten_filetree_create_output.yaml now print values only through this filter.
tests/unit/test_filetree_create_name_injection.py renders every template, loads it back with include_vars and checks the values. A gate parses each template and fails if any printed expression skips the filter.
filetree_create: output paths
Output folders and file names were built from controller names with only / replaced. An organization named .. wrote its export into the parent of output_path. The credentials folder task used the organization name with no replacement, so ../../x created folders outside output_path.
New filter ctrliq.ascender.path_component replaces / and NUL, rewrites ., .. and empty names, and shortens names over 230 bytes with a hash suffix.
Every path in roles/filetree_create/tasks/ built from controller data goes through it, including the credentials folder.
tests/unit/test_filetree_create_paths.py runs the role against a fake API and checks where every file lands.
Inventory plugin: unsafe marking
plugins/inventory/controller.py passed host and group variables from the controller to Ansible as plain strings. Jinja stored in a host or group variable was evaluated on the node running Ansible. Host and group names, variables and controller_metadata are now wrapped with wrap_var.
tests/unit/test_inventory_plugin_unsafe.py feeds the plugin a stubbed API response and checks that the values stay literal. Three of its four tests fail without the fix.
Behavior changes
Exports write booleans and numbers as native YAML instead of quoted strings. Values such as yes or 1.10 keep their string type.
Readers that load exports with plain PyYAML need a constructor for the !unsafe tag. filetree_read handles it already.
Names over 230 bytes, and objects named . or .., get new file names. filetree_read reads file contents, not names, so loading is unaffected.
Templates stored in host or group variables are no longer evaluated when the inventory is used through this plugin. Ascender's own jobs are unaffected.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
All three literal-value tests fail on ansible-core 2.19+ because the untrusted outer template expression remains unevaluated, even when the plugin is correct.
This PR hardens the collection’s controller export and inventory-loading paths against unintended Jinja evaluation, YAML injection, and directory traversal.
Changes:
Adds safe YAML serialization across export templates and the flattening playbook.
Sanitizes controller-derived path components, including special names and oversized filenames.
Marks inventory data unsafe and adds regression tests and changelog entries.
File
Description
tests/unit/test_inventory_plugin_unsafe.py
Tests literal inventory values and preserved structure.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
YAML alias copying can cause exponential memory exhaustion, and the CI unit job installs a package unavailable on its runner, preventing tests from running.
Test unsafe variable marking on older ansible-core versions
plugins/inventory/controller.py:179
The CI version matrix runs only sanity checks, while the pytest job installs the latest ansible-core. On 2.19+, controller data is already untrusted, so these inventory tests pass even if the wrap_var calls are removed. Add a standalone CI run of the unsafe-marking tests on supported ansible-core 2.16–2.18 with a compatible Python version.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Unresolved critical issues can cause CI installation failures, exhaust memory through YAML alias expansion, and let credential paths escape the export root through environment-variable expansion.
Run unsafe-marking regression tests across supported Ansible versions
tests/unit/test_inventory_plugin_unsafe.py:81
On ansible-core 2.19+, these assertions also pass if the inventory plugin's wrap_var calls are removed, because controller values are already untrusted. CI installs the latest core for unit tests (.github/workflows/ci.yml:198); its supported-version matrix runs only sanity checks. Please run this standalone regression suite on supported affected versions such as 2.16 and 2.18 in CI, so the unsafe-marking fix is actually protected against regressions.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
YAML container copying can cause exponential memory growth from shared aliases, and the CI dependency installation can fail on Ubuntu 24.04 runners before tests execute.
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
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.
Reported by Quanah Gibson-Mount @quanah
Summary
This fixes three issues where text stored in the controller can be templated or written somewhere unintended on the machine that runs Ansible. Two were reported externally against
filetree_create. The third, in the controller inventory plugin, was found while validating those reports.Changes
filetree_create: export escaping
The export templates wrote controller strings into YAML without escaping. When
filetree_readloaded the files, Jinja in a name or description was evaluated on the control node. A label namedops{{ 6 * 7 }}came back asops42. A quote and a newline in a value could also add new keys to the exported entry.ctrliq.ascender.export_yamlwrites each value as one line of flow-style YAML. Strings are double-quoted and tagged!unsafe.roles/filetree_create/templates/andplaybooks/flatten_filetree_create_output.yamlnow print values only through this filter.tests/unit/test_filetree_create_name_injection.pyrenders every template, loads it back withinclude_varsand checks the values. A gate parses each template and fails if any printed expression skips the filter.filetree_create: output paths
Output folders and file names were built from controller names with only
/replaced. An organization named..wrote its export into the parent ofoutput_path. The credentials folder task used the organization name with no replacement, so../../xcreated folders outsideoutput_path.ctrliq.ascender.path_componentreplaces/and NUL, rewrites.,..and empty names, and shortens names over 230 bytes with a hash suffix.roles/filetree_create/tasks/built from controller data goes through it, including the credentials folder.tests/unit/test_filetree_create_paths.pyruns the role against a fake API and checks where every file lands.Inventory plugin: unsafe marking
plugins/inventory/controller.pypassed host and group variables from the controller to Ansible as plain strings. Jinja stored in a host or group variable was evaluated on the node running Ansible. Host and group names, variables andcontroller_metadataare now wrapped withwrap_var.tests/unit/test_inventory_plugin_unsafe.pyfeeds the plugin a stubbed API response and checks that the values stay literal. Three of its four tests fail without the fix.Behavior changes
yesor1.10keep their string type.!unsafetag.filetree_readhandles it already..or.., get new file names.filetree_readreads file contents, not names, so loading is unaffected.Testing
pytest --noconftest tests/unit/test_filetree_create_name_injection.py tests/unit/test_filetree_create_paths.py tests/unit/test_inventory_plugin_unsafe.py
ruff checkis clean on the changed plugin and the new test.Changelog fragments:
filetree-create-export-escaping.yml,filetree-create-path-components.yml,inventory-plugin-unsafe.yml.