Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelogs/fragments/10689-gem-prevent-soundness-issue.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
bugfixes:
- "gem - fix soundness issue when uninstalling default gems on Ubuntu (https://github.com/ansible-collections/community.general/issues/10451, https://github.com/ansible-collections/community.general/pull/10689)."
20 changes: 16 additions & 4 deletions plugins/modules/gem.py
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ def uninstall(module):
if module.params['force']:
cmd.append('--force')
cmd.append(module.params['name'])
module.run_command(cmd, environ_update=environ, check_rc=True)
return module.run_command(cmd, environ_update=environ, check_rc=True)


def install(module):
Expand Down Expand Up @@ -334,9 +334,21 @@ def main():
changed = True
elif module.params['state'] == 'absent':
if exists(module):
uninstall(module)
changed = True

command_output = uninstall(module)
if command_output is not None and exists(module):
rc, out, err = command_output
module.fail_json(
msg=(
"Failed to uninstall gem '%s': it is still present after 'gem uninstall'. "
"This usually happens with default or system gems provided by the OS, "
"which cannot be removed with the gem command."
) % module.params['name'],
rc=rc,
stdout=out,
stderr=err
)
else:
changed = True
result = {}
result['name'] = module.params['name']
result['state'] = module.params['state']
Expand Down
15 changes: 15 additions & 0 deletions tests/integration/targets/gem/tasks/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -212,3 +212,18 @@
that:
- install_gem_result is changed
- not gem_bindir_stat.stat.exists

- name: Attempt to uninstall default gem 'json'
community.general.gem:
name: json
state: absent
when: ansible_distribution == "Ubuntu"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good enough for now, but I would suppose this happens because that gem must be installed by default in a .deb package and there is some mechanism in place to prevent the removal. I would suppose other distros/OS'es will have something similar, maybe not by default but after the OS package is installed.

One specific gem in one specific OS feels a bit fragile to me. For the future it might be interesting to better understand this issue and how the OS blocks the removal and either:

  1. ensure blocked-removal gems are installed in different platforms, or
  2. disregard the OS entirely and make the test block a gem and then try to remove it

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My brain coming back to this: I think the main issue I have with this is that the condition for that situation is very likely NOT ansible_distribution == "Ubuntu", but rather that gem located the json gem installed in, say, /usr/lib/ruby/..../ and does not: a) have permissions or, b) allows removing from that location at all or, c) does not allow removing removing from that location without some kind of --force parameter

register: gem_result
ignore_errors: true

- name: Assert gem uninstall failed as expected
when: ansible_distribution == "Ubuntu"
assert:
that:
- gem_result is failed
- gem_result.msg.startswith("Failed to uninstall gem 'json'")
9 changes: 5 additions & 4 deletions tests/unit/plugins/modules/test_gem.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@ def new(module):

def patch_run_command(self):
target = 'ansible.module_utils.basic.AnsibleModule.run_command'
return self.mocker.patch(target)
mock = self.mocker.patch(target)
mock.return_value = (0, '', '')
return mock

def test_fails_when_user_install_and_install_dir_are_combined(self):
with set_module_args({
Expand Down Expand Up @@ -107,12 +109,11 @@ def test_passes_install_dir_and_gem_home_when_uninstall_gem(self):

run_command = self.patch_run_command()

with pytest.raises(AnsibleExitJson) as exc:
with pytest.raises(AnsibleFailJson) as exc:
gem.main()

result = exc.value.args[0]

assert result['changed']
assert result['failed']
assert run_command.called

assert '--install-dir /opt/dummy' in get_command(run_command)
Expand Down