mitogen-hq / mitogen

Distributed self-replicating programs in Python
https://mitogen.networkgenomics.com/
BSD 3-Clause "New" or "Revised" License
2.35k stars 199 forks source link

Fix AnsibleUnsafeText when copying files larger than SMALL_FILE_LIMIT #1110

Closed jrosser closed 2 months ago

jrosser commented 3 months ago

Small files are carried in-band in the communication between controller and remote, with larger files being copied by falling back to transfer_file(). This second code path was missed in b822f20.

Reproducer extracted from original code that failed:

Make the files larger than 2^17 bytes to trigger a failure

- hosts: all
  vars:
    atftpd_path: /srv/tftp
    tftp_ipxe_binaries:
      x86_bios: ipxe/x86_64_undionly.kpxe
      x86_uefi: ipxe/x86_64_ipxe.efi
      arm64_uefi: ipxe/arm64_ipxe.efi
  tasks:
    - copy:
        src: "{{ item.value }}"
        dest: "{{ atftpd_path }}/ipxe_{{ item.key }}.bin"
      with_items:
        - "{{ tftp_ipxe_binaries | dict2items }}"

Exception:

The full traceback is:
Traceback (most recent call last):
  File "/root/venv/lib/python3.10/site-packages/ansible/executor/task_executor.py", line 119, in run
    item_results = self._run_loop(items)
  File "/root/venv/lib/python3.10/site-packages/ansible/executor/task_executor.py", line 334, in _run_loop
    res = self._execute(variables=task_vars)
  File "/root/venv/lib/python3.10/site-packages/ansible/executor/task_executor.py", line 636, in _execute
    result = self._handler.run(task_vars=vars_copy)
  File "/home/ubuntu/mitogen/ansible_mitogen/mixins.py", line 146, in run
    return super(ActionModuleMixin, self).run(tmp, task_vars)
  File "/root/venv/lib/python3.10/site-packages/ansible/plugins/action/copy.py", line 522, in run
    module_return = self._copy_file(source_full, source_rel, content, content_tempfile, dest, task_vars, follow)
  File "/root/venv/lib/python3.10/site-packages/ansible/plugins/action/copy.py", line 305, in _copy_file
    remote_path = self._transfer_file(source_full, tmp_src)
  File "/root/venv/lib/python3.10/site-packages/ansible/plugins/action/__init__.py", line 556, in _transfer_file
    self._connection.put_file(local_path, remote_path)
  File "/home/ubuntu/mitogen/ansible_mitogen/connection.py", line 1129, in put_file
    self.get_chain().call(
  File "/home/ubuntu/mitogen/ansible_mitogen/connection.py", line 466, in call
    return self._rethrow(recv)
  File "/home/ubuntu/mitogen/ansible_mitogen/connection.py", line 452, in _rethrow
    return recv.get().unpickle()
  File "/home/ubuntu/mitogen/mitogen/core.py", line 1009, in unpickle
    raise obj
mitogen.core.CallError: mitogen.core.StreamError: cannot unpickle 'ansible.utils.unsafe_proxy'/'AnsibleUnsafeText'
  File "<stdin>", line 3860, in _dispatch_one
  File "<stdin>", line 3843, in _parse_request
  File "<stdin>", line 998, in unpickle
  File "<stdin>", line 789, in find_class
  File "<stdin>", line 899, in _find_global

fatal: [foobar]: FAILED! => {
    "msg": "Unexpected failure during module execution: mitogen.core.StreamError: cannot unpickle 'ansible.utils.unsafe_proxy'/'AnsibleUnsafeText'\n  File \"<stdin>\", line 3860, in _dispatch_one\n  File \"<stdin>\", line 3843, in _parse_request\n  File \"<stdin>\", line 998, in unpickle\n  File \"<stdin>\", line 789, in find_class\n  File \"<stdin>\", line 899, in _find_global\n",
    "stdout": ""
}
moreati commented 2 months ago

refs #1046

moreati commented 2 months ago

related to #1087. @jrosser thank for these PRs.

  1. May I rebase your branches and add commits to incorporate tests and changlog entries?
  2. Would you like an entry in https://github.com/mitogen-hq/mitogen/blob/master/docs/contributors.rst ?
jrosser commented 2 months ago

@moreati Yes please do rebase / adjust any of my PR as needed to get them suitable to merge. Adding https://github.com/jrosser to the contributors list would be nice - thankyou.

jrosser commented 2 months ago

@moreati If you have a test case for this I can update the branch with your changes as I did in #1087, alternatively we can close this PR and start a new new one from my personal github account so that I can permit you access to the branch.

moreati commented 2 months ago

If you have a test case for this I can update the branch with your changes

In progress.

Notes to self

moreati commented 2 months ago

Like #1087 this only occurs in a with_items: loop, when the destination has a file extension. It does not matter if the source filename has an extension. Unlike #1087 the task immediately fails, and ansible-playbook exits with a non-zero status.

- hosts: localhost
  gather_facts: false
  vars:
    chonkers:
      - {src: ~/tmp/150kiB_seed_1234_v1,      dest: /tmp/150kiB_seed_1234_v1}
      - {src: ~/tmp/150kiB_seed_1234_v1.copy, dest: /tmp/150kiB_seed_1234_v1}
      - {src: ~/tmp/150kiB_seed_1234_v1,      dest: /tmp/150kiB_seed_1234_v1.data}
      - {src: ~/tmp/150kiB_seed_1234_v1.copy, dest: /tmp/150kiB_seed_1234_v1.data}
  tasks:
    - copy:
        src: "{{ item.src }}"
        dest: "{{ item.dest }}"
        mode: u=rw,go=r
      with_items: "{{ chonkers }}"

    - name: Cleanup 1
      file:
        path: "{{ item.dest }}"
        state: absent
      with_items: "{{ chonkers }}"
➜  mitogen git:(master) ✗ git show                                                                                                                                           
commit 84a4fcdf008cd2b90506b5ae4762826c524030bf (HEAD -> master, upstream/master, upstream/HEAD, origin/master, origin/HEAD)
Merge: c95d4112 5ab872f2
Author: Alex Willmer <alex@moreati.org.uk>
Date:   Fri Aug 30 08:02:46 2024 +0100

    Merge pull request #1087 from bbc/unsafe-chmod

    ansible_mitogen: Handle unsafe paths in _remote_chmod

➜  mitogen git:(master) ✗ ANSIBLE_STRATEGY=mitogen_linear ANSIBLE_STRATEGY_PLUGINS=ansible_mitogen/plugins/strategy ~/tmp/v312/bin/ansible-playbook ~/tmp/issue1110_repro.yml
[WARNING]: No inventory was parsed, only implicit localhost is available
[WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does not match 'all'

PLAY [localhost] ***************************************************************************************************************************************************************************************************************************

TASK [copy] ********************************************************************************************************************************************************************************************************************************
ok: [localhost] => (item={'src': '~/tmp/150kiB_seed_1234_v1', 'dest': '/tmp/150kiB_seed_1234_v1'})
ok: [localhost] => (item={'src': '~/tmp/150kiB_seed_1234_v1.copy', 'dest': '/tmp/150kiB_seed_1234_v1'})
ERROR! [mux  68294] 09:48:51.595685 E mitogen.[local.68296]: raw pickle was: b'\x80\x02(X!\x00\x00\x00kintha-68295-1f78b8f40-548a2df07fq\x00X\x16\x00\x00\x00ansible_mitogen.targetq\x01NX\r\x00\x00\x00transfer_fileq\x02)cmitogen.core\nKwargs\nq\x03}q\x04(X\x07\x00\x00\x00contextq\x05cmitogen.core\n_unpickle_context\nq\x06K\x00N\x86q\x07Rq\x08X\x07\x00\x00\x00in_pathq\tX#\x00\x00\x00/Users/alex/tmp/150kiB_seed_1234_v1q\nX\x08\x00\x00\x00out_pathq\x0bcansible.utils.unsafe_proxy\nAnsibleUnsafeText\nq\x0cXM\x00\x00\x00/Users/alex/.ansible/tmp/ansible_mitogen_action_adffb98f18aac450/.source.dataq\r\x85q\x0eRq\x0fu\x85q\x10Rq\x11tq\x12.'
An exception occurred during task execution. To see the full traceback, use -vvv. The error was:   File "<stdin>", line 899, in _find_global
fatal: [localhost]: FAILED! => 
  msg: |-
    Unexpected failure during module execution: mitogen.core.StreamError: cannot unpickle 'ansible.utils.unsafe_proxy'/'AnsibleUnsafeText'
      File "<stdin>", line 3860, in _dispatch_one
      File "<stdin>", line 3843, in _parse_request
      File "<stdin>", line 998, in unpickle
      File "<stdin>", line 789, in find_class
      File "<stdin>", line 899, in _find_global
  stdout: ''

PLAY RECAP *********************************************************************************************************************************************************************************************************************************
localhost                  : ok=0    changed=0    unreachable=0    failed=1    skipped=0    rescued=0    ignored=0   

➜  mitogen git:(master) ✗ echo $?
2
moreati commented 2 months ago

Confirm that the loop driven version of tests/ansible/integration/copy.yml in https://github.com/moreati/mitogen/commit/ce1accedbce1d841d2fa88676fabbdaa1487f92f fails if all loop: keywords are replaced by with_items: and this PR has not been applied.

TASK [Copy sourced files src={{ item.src }}, dest={{ item.dest }}, mode=u=rw,go=r] ***********************************
Friday 30 August 2024  15:57:00 +0000 (0:00:00.059)       0:00:18.424 ********* 
changed: [target-debian10-1] => (item=/tmp/copy-tiny-file.out)
changed: [target-debian11-2] => (item=/tmp/copy-tiny-file.out)
changed: [target-ubuntu2004-3] => (item=/tmp/copy-tiny-file.out)
ERROR! [mux  140446] 15:57:00.997443 E mitogen.[ssh.localhost:2202]: raw pickle was: b'\x80\x02(X%\x00\x00\x00uv2404-142265-721cfe0e1080-18e27186e7q\x00X\x16\x00\x00\x00ansible_mitogen.targetq\x01NX\r\x00\x00\x00transfer_fileq\x02)cmitogen.core\nKwargs\nq\x03}q\x04(X\x07\x00\x00\x00contextq\x05cmitogen.core\n_unpickle_context\nq\x06K\x00N\x86q\x07Rq\x08X\x07\x00\x00\x00in_pathq\tX\x14\x00\x00\x00/tmp/copy-large-fileq\nX\x08\x00\x00\x00out_pathq\x0bcansible.utils.unsafe_proxy\nAnsibleUnsafeText\nq\x0cX]\x00\x00\x00/home/mitogen__has_sudo_nopw/.ansible/tmp/ansible_mitogen_action_78b7c26ed6a4d40c/.source.outq\r\x85q\x0eRq\x0fu\x85q\x10Rq\x11tq\x12.'
ERROR! [mux  140446] 15:57:01.000550 E mitogen.[ssh.localhost:2201]: raw pickle was: b'\x80\x02(X%\x00\x00\x00uv2404-142264-721cfe0e1080-18e27183daq\x00X\x16\x00\x00\x00ansible_mitogen.targetq\x01NX\r\x00\x00\x00transfer_fileq\x02)cmitogen.core\nKwargs\nq\x03}q\x04(X\x07\x00\x00\x00contextq\x05cmitogen.core\n_unpickle_context\nq\x06K\x00N\x86q\x07Rq\x08X\x07\x00\x00\x00in_pathq\tX\x14\x00\x00\x00/tmp/copy-large-fileq\nX\x08\x00\x00\x00out_pathq\x0bcansible.utils.unsafe_proxy\nAnsibleUnsafeText\nq\x0cX]\x00\x00\x00/home/mitogen__has_sudo_nopw/.ansible/tmp/ansible_mitogen_action_92f38c8edfa56781/.source.outq\r\x85q\x0eRq\x0fu\x85q\x10Rq\x11tq\x12.'
ERROR! [mux  140446] 15:57:01.007620 E mitogen.[ssh.localhost:2203]: raw pickle was: b'\x80\x02(X%\x00\x00\x00uv2404-142266-721cfe0e1080-18e271a183q\x00X\x16\x00\x00\x00ansible_mitogen.targetq\x01NX\r\x00\x00\x00transfer_fileq\x02)cmitogen.core\nKwargs\nq\x03}q\x04(X\x07\x00\x00\x00contextq\x05cmitogen.core\n_unpickle_context\nq\x06K\x00N\x86q\x07Rq\x08X\x07\x00\x00\x00in_pathq\tX\x14\x00\x00\x00/tmp/copy-large-fileq\nX\x08\x00\x00\x00out_pathq\x0bcansible.utils.unsafe_proxy\nAnsibleUnsafeText\nq\x0cX]\x00\x00\x00/home/mitogen__has_sudo_nopw/.ansible/tmp/ansible_mitogen_action_19afbf54168d85d7/.source.outq\r\x85q\x0eRq\x0fu\x85q\x10Rq\x11tq\x12.'
An exception occurred during task execution. To see the full traceback, use -vvv. The error was:   File "<stdin>", line 899, in _find_global
fatal: [target-debian11-2]: FAILED! => 
  msg: |-
    Unexpected failure during module execution: mitogen.core.StreamError: cannot unpickle 'ansible.utils.unsafe_proxy'/'AnsibleUnsafeText'
      File "<stdin>", line 3860, in _dispatch_one
      File "<stdin>", line 3843, in _parse_request
      File "<stdin>", line 998, in unpickle
      File "<stdin>", line 789, in find_class
      File "<stdin>", line 899, in _find_global
  stdout: ''
An exception occurred during task execution. To see the full traceback, use -vvv. The error was:   File "<stdin>", line 899, in _find_global
fatal: [target-debian10-1]: FAILED! => 
  msg: |-
    Unexpected failure during module execution: mitogen.core.StreamError: cannot unpickle 'ansible.utils.unsafe_proxy'/'AnsibleUnsafeText'
      File "<stdin>", line 3860, in _dispatch_one
      File "<stdin>", line 3843, in _parse_request
      File "<stdin>", line 998, in unpickle
      File "<stdin>", line 789, in find_class
      File "<stdin>", line 899, in _find_global
  stdout: ''
An exception occurred during task execution. To see the full traceback, use -vvv. The error was:   File "<stdin>", line 899, in _find_global
fatal: [target-ubuntu2004-3]: FAILED! => 
  msg: |-
    Unexpected failure during module execution: mitogen.core.StreamError: cannot unpickle 'ansible.utils.unsafe_proxy'/'AnsibleUnsafeText'
      File "<stdin>", line 3860, in _dispatch_one
      File "<stdin>", line 3843, in _parse_request
      File "<stdin>", line 998, in unpickle
      File "<stdin>", line 789, in find_class
      File "<stdin>", line 899, in _find_global
  stdout: ''
moreati commented 2 months ago

@jrosser I think https://github.com/moreati/mitogen/tree/unsafe-large-copy is ready for you to update your branch with. It turns out the test suite already had tests for large file copies, but they weren't inside a with_items: loop so didn't catch this.