diff --git a/changelogs/fragments/64092-get_url_verify_tmpsrc_checksum.yml b/changelogs/fragments/64092-get_url_verify_tmpsrc_checksum.yml new file mode 100644 index 00000000000..8f650175411 --- /dev/null +++ b/changelogs/fragments/64092-get_url_verify_tmpsrc_checksum.yml @@ -0,0 +1,2 @@ +bugfixes: + - get_url - Verify checksum using tmpsrc, not dest (https://github.com/ansible/ansible/pull/64092) diff --git a/lib/ansible/modules/get_url.py b/lib/ansible/modules/get_url.py index d7c1cc45d15..959998c9591 100644 --- a/lib/ansible/modules/get_url.py +++ b/lib/ansible/modules/get_url.py @@ -663,6 +663,16 @@ def main(): result['checksum_src'] != result['checksum_dest']) module.exit_json(msg=info.get('msg', ''), **result) + # If a checksum was provided, ensure that the temporary file matches this checksum + # before moving it to the destination. + if checksum != '': + tmpsrc_checksum = module.digest_from_file(tmpsrc, algorithm) + + if checksum != tmpsrc_checksum: + os.remove(tmpsrc) + module.fail_json(msg=f"The checksum for {tmpsrc} did not match {checksum}; it was {tmpsrc_checksum}.", **result) + + # Copy temporary file to destination if necessary backup_file = None if result['checksum_src'] != result['checksum_dest']: try: @@ -681,13 +691,6 @@ def main(): if os.path.exists(tmpsrc): os.remove(tmpsrc) - if checksum != '': - destination_checksum = module.digest_from_file(dest, algorithm) - - if checksum != destination_checksum: - os.remove(dest) - module.fail_json(msg="The checksum for %s did not match %s; it was %s." % (dest, checksum, destination_checksum), **result) - # allow file attribute changes file_args = module.load_file_common_arguments(module.params, path=dest) result['changed'] = module.set_fs_attributes_if_different(file_args, result['changed']) diff --git a/test/integration/targets/get_url/tasks/main.yml b/test/integration/targets/get_url/tasks/main.yml index 51cb437b6af..2f50b4366c1 100644 --- a/test/integration/targets/get_url/tasks/main.yml +++ b/test/integration/targets/get_url/tasks/main.yml @@ -676,3 +676,46 @@ - name: Test use_netrc=False import_tasks: use_netrc.yml + +# https://github.com/ansible/ansible/pull/64092 +# Calling get_url with bad checksum should not delete the target file +- name: Define test files for checksum verification + set_fact: + checksum_verify_dstfile: "{{ remote_tmp_dir }}/checksum-verify-test.txt" + +- name: Download file + get_url: + url: https://{{ httpbin_host }}/get + dest: "{{ checksum_verify_dstfile}}" + register: result + +- stat: + path: "{{ checksum_verify_dstfile }}" + register: stat_result_checksum_verify + +- name: Assert success + assert: + that: + - result is changed + - '"OK" in result.msg' + - stat_result_checksum_verify.stat.exists + +- name: Download file again, with wrong checksum + get_url: + url: https://{{ httpbin_host }}/get + dest: "{{ checksum_verify_dstfile}}" + checksum: "sha256:18b2a70b53c350ad49e4eafb69560bf77ba2ef4f3c93376b65f18b753c912809" + register: result + failed_when: + - result is successful + +- stat: + path: "{{ checksum_verify_dstfile }}" + register: stat_result_checksum_verify + +- name: Assert destination file was not removed + assert: + that: + - result is not changed + - '"did not match" in result.msg' + - stat_result_checksum_verify.stat.exists