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

command module: environment variables are substituted unconditionally #54162

Closed
rfc1459 opened this issue Mar 21, 2019 · 16 comments · Fixed by #80512
Closed

command module: environment variables are substituted unconditionally #54162

rfc1459 opened this issue Mar 21, 2019 · 16 comments · Fixed by #80512
Assignees
Labels
affects_2.15 affects_2.16 bug This issue/PR relates to a bug. commands Commands category feature This issue/PR relates to a feature request. has_pr This issue has an associated PR. module This issue/PR relates to a module. support:core This issue/PR relates to code supported by the Ansible Engineering Team.

Comments

@rfc1459
Copy link

rfc1459 commented Mar 21, 2019

SUMMARY

Contrary to what is stated in the module documentation, all occurrences of environment variables are unconditionally expanded. The expansion also does not take escape characters into account, so \$HOME gets expanded to \actual-home-directory-path.

This happens because expand_user_and_vars is not set to False when calling module.run_command and there is no way to control this behavior.

ISSUE TYPE
  • Feature request
COMPONENT NAME

command module

ANSIBLE VERSION
$ ansible --version
ansible 2.7.9
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/home/administrator/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/dist-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.15rc1 (default, Nov 12 2018, 14:31:15) [GCC 7.3.0]
CONFIGURATION

N/A

OS / ENVIRONMENT

N/A

STEPS TO REPRODUCE

Minimal test case:

$ ansible -m command -a 'ls $HOME' localhost
EXPECTED RESULTS

Command should fail with result ls: cannot access '$HOME': No such file or directory

ACTUAL RESULTS

https://gist.github.com/rfc1459/0f80f01d0c93e73f27dfdd64f0069b0c

@ansibot
Copy link
Contributor

ansibot commented Mar 21, 2019

Files identified in the description:

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibot ansibot added affects_2.7 This issue/PR affects Ansible v2.7 bug This issue/PR relates to a bug. commands Commands category module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Mar 21, 2019
@bcoca
Copy link
Member

bcoca commented Mar 22, 2019

consider you are already consuming one quote due to calling ansible in a shell ansible -m command -a "'ls $HOME'" localhost

@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label Mar 22, 2019
@rfc1459
Copy link
Author

rfc1459 commented Mar 22, 2019

Shell quotes with ansible command are a red herring (wrapping in double quotes as you wrote actually makes the shell expand $HOME), I reproduced the issue with a minimal playbook as well:

---
- hosts: localhost
  tasks:
    - name: Should not expand $HOME
      command:
        /usr/bin/stat $HOME
      register: dont_expand_home
      failed_when: dont_expand_home.rc == 0

This still fails: https://gist.github.com/rfc1459/fd6b498ad6e4537fb0fd9f51757b6aa2.

This is caused by module.run_command() calling os.path.expandvars which expands all environment variables it encounters without taking into account any escape characters or single quotes (basically, the same issue as #52585). Using the shell module and wrapping $HOME in single quotes works because module.run_command() is called with unsafe_shell=True, so the call to os.path.expandvars is skipped.

@rfc1459 rfc1459 changed the title command module: path-like shell variables are substituted unconditionally command module: environment variables are substituted unconditionally Mar 22, 2019
@rfc1459
Copy link
Author

rfc1459 commented Mar 22, 2019

Actually, looking at the source of os.path.expandvars all environment variables are unconditionally expanded, not just $HOME and similar vars. I updated the issue title and description accordingly.

@JacobHenner
Copy link

Reproduced with Ansible 2.5.5

@fpob
Copy link

fpob commented Jul 29, 2019

Actually variables are expanded even in other modules where no one expect variables expansion. For example

- git:
    repo: file:///tmp/$HOME
    dest: /tmp/repo

- user:
    name: test
    comment: $HOME

- unarchive:
    src: /tmp/$HOME
    dest: /tmp

- acl:
    path: $HOME

And there is more modules that use module.run_command().

@bcoca
Copy link
Member

bcoca commented Jul 30, 2019

@fpob all file paths will get expanded, that is expected and done on purpose.

In other cases it depends on being backed by an API or by a CLI tool, which is the case for user and git

@fpob
Copy link

fpob commented Jul 30, 2019

@bcoca paths yes, it is expected. Unexpected is env variable expansion.

@yewwayne
Copy link

Still an issue in 2.9.6. There's no way to pass a literal $HOME using command module.

@bcoca
Copy link
Member

bcoca commented Mar 26, 2020

no need to update, while ticket is open the issue is supposed to persist, leaning to this not being a bug, since the substitution is intended in all the cases discussed.

I have not really seen a use case for the variables not to be expanded, just demonstrations on when they are.

@rfc1459
Copy link
Author

rfc1459 commented Mar 26, 2020

Yet the documentation says otherwise:

The command(s) will not be processed through the shell, so variables like $HOME and operations like "<", ">", "|", ";" and "&" will not work. Use the shell module if you need these features.

It all boils down to whether the documentation is correct (then this is definitely a bug) or wrong (then it should be very clear that environment variables will be expanded unconditionally whether you want it or not).

By the way, here's the original use case: I need to send a command to a remote host which needs a literal $HOME in its arguments, but the command module will expand it unconditionally before calling the command, which is neither what I want nor what's written in the module docs.

@fpob
Copy link

fpob commented Apr 16, 2020

@bcoca OK, let's say variable expansion is expected.

What about using string.Template(...).safe_substitute(os.environ) (https://docs.python.org/3/library/string.html#template-strings) instead of os.path.expandvar(...)? It should work the same way but it will allow to escape $.


@rfc1459 If you just want use literal $HOME in command you can try set HOME to $HOME in environment on task. Tried it on minimal playbook from your previous comment;

- hosts: localhost
  tasks:
    - name: Should not expand $HOME
      command:
        /usr/bin/stat $HOME
      register: dont_expand_home
      failed_when: dont_expand_home.rc == 0
      environment:
        HOME: $HOME

@ansibot
Copy link
Contributor

ansibot commented May 16, 2020

Files identified in the description:

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

@riyad
Copy link
Contributor

riyad commented Sep 19, 2020

I just stepped on this mine also ... and wasted at least 1h trying to find the right incantations: without quotes, with quotes, nesting quotes, escaping quotes, using argv with all of the above, because nobody expects the spani... this behavior.

In my case I wanted to run resolvectl domain lxdbr0 '~lxd'. Note how there's a literal "~" in the last argument! It is the indicator of a routing-only domain (for DNS!) and definitely not meant to be interpreted as "home directory of user lxd" (which exists, and therefore triggers this bug).

It drove me nuts:

  • quotes are stripped (not ideal, but tolerable if arguments are passed on correctly)
  • things looking like paths are expanded 😳
  • but documentation says that doesn't happen 😖

The current behavior is definitely at odds with the documentation. I always expected shell: ... to be parsed and mangled (at least it respects quoting), but command: ... was supposed to be dumber (i.e. "safer") smaller brother of it ... something sort of exec*()-like.

I see the following possible solutions:

  • Fix the documentation by codifying the status quo
  • Fix the module to match the documentation
  • Add a flag to be able to switch between both behaviors (which the underlying APIs seem to allow)

@s-hertel s-hertel added affects_2.12 and removed affects_2.7 This issue/PR affects Ansible v2.7 labels Sep 10, 2021
@s-hertel
Copy link
Contributor

I updated this to be a feature request instead of a bug report because we may want to address this by adding a new option to the command module to be backwards compatible.

@ansibot ansibot added the feature This issue/PR relates to a feature request. label Sep 10, 2021
@jborean93
Copy link
Contributor

While we can't change the behaviour a new option to disable it is being proposed by #80512.

@ansibot ansibot added the has_pr This issue has an associated PR. label Apr 12, 2023
@ansible ansible locked and limited conversation to collaborators May 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.15 affects_2.16 bug This issue/PR relates to a bug. commands Commands category feature This issue/PR relates to a feature request. has_pr This issue has an associated PR. module This issue/PR relates to a module. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants