get_url: Verify checksum using tmpsrc, not dest (#64092)

Previously, if the checksum of the downloaded file did not match the
specified checksum, the *destination* file was removed. This possibly
leaves the system that is being provisioned in an invalid state.

Instead, the checksum should be calculated on the temporary file only.
If there's a mismatch, delete the *temporary* file, not the destination
file.

This requires checking the checksum before moving the file.

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)

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'])

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