Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request fixes a security vulnerability where Ansible inventory variables (including sensitive data like certificates and tokens) were being exposed as Rundeck node attributes regardless of configuration settings. The fix centralizes the filtering logic for Ansible special variables and ensures the importInventoryVars configuration is properly respected in both code paths (gather facts and inventory list).
Changes:
- Introduced a centralized
ANSIBLE_SPECIAL_VARSlist to consistently filter Ansible magic variables across the codebase - Fixed a bug in
createNodeEntrywhere inventory variables were always imported even whenimportInventoryVarswas false - Added comprehensive test coverage for variable filtering, including edge cases with multiple prefixes and malformed input
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| AnsibleResourceModelSource.java | Adds centralized ANSIBLE_SPECIAL_VARS list and fixes createNodeEntry to respect importInventoryVars setting with consistent filtering logic |
| AnsibleResourceModelSourceSpec.groovy | Adds comprehensive unit tests for filtering behavior including edge cases, and updates existing test to explicitly enable importInventoryVars |
| YamlParsingSpec.groovy | Updates assertions to verify ansible_ prefixed variables are filtered while custom variables are imported |
| InventoryListSpec.groovy | Updates assertions to verify ansible_ prefixed variables are filtered while custom variables are imported |
| project.properties (yaml-parsing) | Adds ansible-import-inventory-vars=true to maintain test functionality after bug fix |
| project.properties (large-inventory) | Adds ansible-import-inventory-vars=true to maintain test functionality after bug fix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.
JIRA TICKET: https://pagerduty.atlassian.net/browse/RUN-4032
This pull request updates how Ansible inventory variables are imported as node attributes, ensuring that Ansible special variables (such as those prefixed with
ansible_) are filtered out and not included in node attributes. The changes also improve configurability and consistency in handling these variables across different code paths, and update the related tests and test configurations accordingly.This fix prevents sensitive inventory variables from being exposed as Rundeck node attributes, addressing the customer's security concern about certificates, tokens, and credentials being visible in the UI.
Important Change: Projects relying on the buggy behavior (expecting Ansible variables in attributes when importInventoryVars is not explicitly configured) will need to enable "Import host vars" in their
configuration.
Rationale:
Filtering and handling of Ansible inventory variables:
ANSIBLE_SPECIAL_VARSlist inAnsibleResourceModelSource.javato define Ansible special variables that should be excluded from node attributes. This list is now used consistently in bothprocessWithGatherFactsandcreateNodeEntrymethods to filter out these variables. [1] [2] [3]createNodeEntryto skip variables matching the ignore list, aligning its behavior withprocessWithGatherFactsfor inventory variable filtering. [1] [2]Test updates and configuration:
InventoryListSpec.groovy,YamlParsingSpec.groovy, andAnsibleResourceModelSourceSpec.groovy) to assert that Ansible special variables are no longer present in node attributes, while custom variables are still imported. [1] [2] [3] [4]ansible-import-inventory-vars=trueproperty to relevant test project properties to ensure inventory variables are imported during tests. [1] [2]