ansible/molecule

Use of autoescape=True in Jinja template breaks Vagrant

samiam opened this issue · 4 comments

Issue Type

  • Bug report

Molecule and Ansible details

molecule 5.0.1 using python 3.11
    ansible 2.14.5
    delegated 5.0.1 from molecule

Molecule installation method (one of):

  • pip

Ansible installation method (one of):

  • pip

Detail any linters or test runners used:

Desired Behavior

Jinja2 should generate correct Vagrant config files as it did in molecule <= v4.

Actual Behavior

This line:
https://github.com/ansible-community/molecule/blob/main/src/molecule/util.py#L178
from this PR introduced the regression.

Briefly, quoted strings that used to be output in the Vagrantfile, are now escaped.

# v4
c.vm.box_url = "https://some.url.com"

# v5
c.vm.box_url = &#34;https://some.url.com&#34;

molecule-vagrant no longer works.

Here's a line from the template that I'm using as an example. There's plenty more in there.

https://github.com/ansible-community/molecule-vagrant/blob/main/molecule_vagrant/modules/vagrant.py#L222

Here's my PR that adds a simple test of quoted strings in Jinja2 templates:
#3911

Summary

My ask is that autoescape be turned off when rendering templates.

I hoped I fixed everything in ansible-community/molecule-plugins@7eab71d. Looks like it's not the case. Can you open a bug on molecule-plugins with:

  • a test case
  • the parts of the template missing escaping ?

I'm not sure that disabling auto-escaping in molecule will happen since on general case it's a matter of security, so it's better to fix the template. If/When ansible-community/molecule-plugins#142 is merged, it may be considered to disable auto-escaping since it won't impact molecule.

I was afraid of this response. 😄

I'm not sure I buy the security argument since this is a testing harness and templates are used to generate files but I'm not going to die on that hill.

I can look at the template. I already started and did exactly what you did; add the safe filter. I can add them to the entire template wherever vars are used, but then, what's the point? We'd just be undoing the auto-escape feature.

Plus, there are vars that come from config options, like provider_raw_config_args that may or may not have double quotes, so you just have to assume there could be quotes and pass that to safe as well. eg.

 - "customize ['createmedium', 'disk', '--filename', 'example', '--size', '2048']"

Ref: ansible-community/molecule-plugins#60

Perhaps a middle ground is to create a new function render_template_unsafe() which won't pass the auto-escape flag, or the reverse - keep current behavior and create new function render_template_safe(). Or pass autoescape as an option to the current render_template.

Lastly, the Jinja docs seem to reference this very issue:
https://jinja.palletsprojects.com/en/3.0.x/templates/#working-with-automatic-escaping

If #142 is going to get merged soon and we can go back to old behavior, that may save a lot of headache for everyone.

I added a PR with the additional safe filters:
ansible-community/molecule-plugins#153.

Sorry I don't have a test with it.
I'm new to tox and my environment wasn't working.
I wish there was a little guidance on setting up the tests to run...
Anyway, I'll try to get one written when I can.
It will probably be a little janky; reading the Vagrantfile for certain config statements to ensure they are correct?

@apatard

Here's the issue I opened in molecule-plugins:
ansible-community/molecule-plugins#158

In the PR you'll find a fix and test to the autoescape issue.