Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(MODULES-11074) Fix facts_diff task argument parsing on Windows #561

Merged
merged 1 commit into from
May 18, 2021

Conversation

luchihoratiu
Copy link
Contributor

@luchihoratiu luchihoratiu commented May 17, 2021

Previously, running the facts_diff task with its exclude argument set to a regular expression with characters considered special in cmd/powershell/bash could end up discarded or injected as part of the underlying puppet facts diff CLI command.

This commit ensures that all cmd/powershell/bash special characters are escaped by wrapping the exclude regular expression argument content between double quotes and validates it before passing it to the Puppet::Util::Execution.execute method. Invalid regular expressions will raise RegexpError.

Used ^!@%\$&(a)d+\w*\.\/|facter|\|d?[a-z]-=<>;:|a{3}|0`~|\(\[\<$|custom as a base for testing the exclude argument against special characters. Manual tests passed using CLI, PE console and bolt on both Linux and Windows machines.

Tests included adding custom facts that contained such special characters to ensure excluding mechanism still works as expected. Some examples:

Facter.add('|custom') do
  setcode do
    Facter.value(:facterversion)
  end
end

Facter.add('c|ustom') do
  setcode do
    Facter.value(:facterversion)
  end
end

Facter.add('!custom') do
  setcode do
    Facter.value(:facterversion)
  end
end

Known issues

Ideally, we should be able to give the same input to all 3 methods mentioned (PE console, bolt and CLI), as seen below:
Untitled Diagram (8)

But there are some limitations:

  1. All regular expressions need to be accordingly escaped when running puppet facts diff or bolt task run puppet_agent::facts_diff depending on the shell limitations.

  2. Regular expressions starting with double quote (") need to be escaped using backslash (\) in PE console due to puppetserver nature of dealing with strings.

With above limitations in mind, the initial regular expression was also combined with user query given in ticket, resulting in ^!@%\$&(a)d+\w*\.\/|facter|\|d?[a-z]-=<>;:|a{3}|0`~|\(\[\<$|custom|puppet_agent_pid|puppet_inventory_metadata|processors\.|disks.*type|os\.distro|lsb.*release|dhcp_servers\.system|hypervisors\.|ec2_userdata|networking.*\.scope6|pe_postgresql_info|docker\.SystemTime|docker\.Swarm\.RemoteManagers|docker\.NGoroutines and everything worked as expected.

@luchihoratiu luchihoratiu requested a review from a team as a code owner May 17, 2021 14:02
@luchihoratiu luchihoratiu requested a review from a team May 17, 2021 14:05
Previously, running the `facts_diff` task with its `exclude` argument
set to a regular expression with characters considered special in
`cmd`/`powershell`/`bash` could end up discarded or injected as part of
the underlying `puppet facts diff` CLI command.

This commit ensures that all `cmd`/`powershell`/`bash` special
characters are escaped by wraping the `exclude` regular expression
argument content between double quotes and validates it before passing
it to the `Puppet::Util::Execution.execute` method.
@mihaibuzgau mihaibuzgau merged commit 167fb44 into puppetlabs:main May 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants