The imp module has been deprecated for quite some time. The importlib
module has been present since python 3.6 and replaces the imp module.
This is important because python3.12 removes the imp module entirely and
any code using imp needs to be rewritten to be python3.12 compatible.
The rewrite here comes from the suggestions in an upstream discussion
about this transition [0].
[0] https://discuss.python.org/t/how-do-i-migrate-from-imp/27885
Change-Id: Iaa742c71bbb208225726c2b8fa5af111f48667c9
If a role is applied to a host more than once (via either play
roles or include_roles, but not via an include_role loop), it will
have the same task UUID from ansible which means Zuul's command
plugin will write the streaming output to the same filename, and
the log streaming will request the same file. That means the file
might look this after the second invocation:
2022-05-19 17:06:23.673625 | one
2022-05-19 17:06:23.673781 | [Zuul] Task exit code: 0
2022-05-19 17:06:29.226463 | two
2022-05-19 17:06:29.226605 | [Zuul] Task exit code: 0
But since we stop reading the log after "Task exit code", the user
would see "one" twice, and never see "two".
Here are some potential fixes for this that don't work:
* Accessing the task vars from zuul_stream to store any additional
information: the callback plugins are not given the task vars.
* Setting the log id on the task args in zuul_stream instead of
command: the same Task object is used for each host and therefore
the command module might see the task object after it has been
further modified (in other words, nothing host-specific can be
set on the task object).
* Setting an even more unique uuid than Task._uuid on the Task
object in zuul_stream and using that in the command module instead
of Task._uuid: in some rare cases, the actual task Python object
may be different between the callback and command plugin, yet still
have the same _uuid; therefore the new attribute would be missing.
Instead, a global variable is used in order to transfer data between
zuul_stream and command. This variable holds a counter for each
task+host combination. Most of the time it will be 1, but if we run
the same task on the same host again, it will increment. Since Ansible
will not run more than one task on a host simultaneously, so there is
no race between the counter being incremented in zuul_stream and used
in command.
Because Ansible is re-invoked for each playbook, the memory usage is
not a concern.
There may be a fork between zuul_stream and command, but that's fine
as long as we treat it as read-only in the command plugin. It will
have the data for our current task+host from the most recent zuul_stream
callback invocation.
This change also includes a somewhat unrelated change to the test
infrastructure. Because we were not setting the log stream port on
the executor in tests, we were actually relying on the "real" OpenDev
Zuul starting zuul_console on the test nodes rather than the
zuul_console we set up for each specific Ansible version from the tests.
This corrects that and uses the correct zuul_console port, so that if we
make any changes to zuul_console in the future, we will test the
changed version, not the one from the Zuul which actually runs the
tox-remote job.
Change-Id: Ia656db5f3dade52c8dbd0505b24049fe0fff67a5
There's actually not anything unsafe about add_host. Doing CD from Zuul
requires being able to add hosts, so relax the restriction.
Change-Id: I7a5992808773722f3b81890fb4193da202cfea68
After reworking the injection of zuul_log_id this is no longer
necessary.
This reverts commit dec1900178.
This reverts commit d6f1fd13fa.
Change-Id: I2c491e0f643638ef57ef86f505974fb012939569
The log streaming callback is not being called in the same way
in Ansible 2.5 as it was in 2.3. In particular, in some cases
different Task objects are used for different hosts. This,
combined with the fact that the callback is only called once for
a given task means that in these cases we are unable to supply
the zuul_log_id to the Task object for the second host on a task.
This can be resolved by injecting the zuul_log_id within the command
action plugin based on the task uuid directly.
Change-Id: I7ff35263c52d93aeabe915532230964994c30850
Whitelist zuul_return to allow untrusted jobs to run the task on the
executor (localhost). Otherwise, only trusted jobs are only able to
use it.
Change-Id: I768394251d7a2ee102883694bfc93845254e8514
Signed-off-by: Paul Belanger <pabelanger@redhat.com>
The log streaming callback is not being called in the same way
in Ansible 2.5 as it was in 2.3. In particular, in some cases
different Task objects are used for different hosts. This,
combined with the fact that the callback is only called once for
a given task means that in these cases we are unable to supply
the zuul_log_id to the Task object for the second host on a task.
To correct this, a local copy of the linear strategy plugin is
added, with the change that for every host-task, it calls either
the normal on_task_start callback, or a new zuul_task_start
callback. This ensures that we are able to set up log streaming
on every host-task.
We plan to move to a different system for establishing log streaming
soon so that we don't have to keep carrying this patched plugin.
Story: 2002528
Task: 22067
Change-Id: Ifd17d799fc28174df5194a6cd09e0e25f3ea75ac
Co-Authored-By: Tobias Henkel <tobias.henkel@bmw-carit.de>
Co-Authored-By: Clark Boylan <clark.boylan@gmail.com>
Currently when a job adds a zuul role repo to a playbook, we only
use the master branch of the role repo, unless the role repo
appears in the dependency chain for the change under test.
That means that if the role repo appears in required-projects,
but not as a dependency, then we use the master branch instead of
what was specified in required-projects. That doesn't seem to make
much sense and is likely an oversight. We attempt to use the
prepared repos where possible (ie, the requested branches match
and the playbook is not trusted). However, the current check for
that only looks at 'items', that is, the dependency chain. Instead,
we should look at 'projects', which includes not only the projects
which appear in 'items', but also those that appear in
required-projects.
The same check is performed for playbooks, and therefore is also
updated.
Also, in the case where a role repo doesn't appear in either the
dependency chain or in required-projects, we were hard-coded to
check out the 'master' branch. Instead, re-use some of the logic
used when preparing required-projects to attempt to find the best
branch to check out. We will try the job override branch first,
then the zuul branch, then the project default branch.
All playbook project repos are now prepared outside of the work dir,
even in cases where their projects also appear in the work dir. If
the playbook is untrusted, then the repo is cloned into the "untrusted/"
jobdir directory (with speculative changes applied). To account for
this, the "allow_trusted" flag in the ansible safe path checker is
updated to allow access to both "trusted/" and "untrusted/" paths.
Change-Id: If95a9b0aaff982040cd4e6e957f9588b26ef7935
Currently the lookup plugins are only permitted to read data
from the work dir. Some of them expect to read files based on
the CWD which Ansible sets to be the playbook directory. In the
case that the lookup plugins are being run by a playbook in a
project which is in the dependency chain, that will work fine.
But if the playbook project is not in the dependency chain, it
is checked out into the "trusted/" directory in the jobdir, which
is outside of the work dir. In this case, the lookup plugin will
fail.
Alter the lookup plugins to permit access to the "trusted/"
directory as well as "work/". This is safe because these plugins
only read files, not write. And the only content in the "trusted/"
directory is git repository checkouts which the user already has
access to.
Also allow the include_vars and synchronize action modules access
to read files from these directories.
The current tests are sufficient to show that current behavior is
not broken by this change. A followup change moves all playbook
project checkouts outside of the work directory, so that case will
be tested then.
Change-Id: Ie2e5b0d1c099d4f9cf59c1e67fb0603d7c5f757b
Currently we don't check some modules for delegation to
localhost. This would make it possible to overwrite any data which is
writable within the bwrap context. Further the script module allows
arbitrary code execution when delegated to localhost.
The following modules are affected:
* assemble: add safe path check
* copy: add safe path check
* patch: add safe path check
* script: block completely
* template: add safe path check
* unarchive: add tests, fixed by safe path check of copy
Change-Id: I2360219f50e6a28bb134468ead08ec72148ad192
Story: 2001681
Currently it is possible to bypass safe path checks by utilising
modules that can operate on directories instead of files like
assemble. This can be done by putting symlinks into a directory the
module is allowed to access.
This can be fixed by walking the whole sub tree and checking the paths
instead of just checking the path itself.
Change-Id: Iaa4efcf0737e47429339e9afd66eecf4e38fd8ea
In ansible find_needle is used for finding source files. This must be
allowed also for roles from trusted repos.
Change-Id: I0491bc08ba1869849a562bd5047253e60c40c7d7
Currently the safe path checks mostly just work on the pure given
arguments. However ansible searches in several paths for the file and
uses this then. Thus the safe path check can validate a non existing
file (as it doesn't obey the modules search path) while the module
finds the file and follows any symlinks.
The solution for this is not to try to determine the correct paths
ourselves but hooking into the search routine (_find_needle) the
modules use for finding their local files. Using this approach we have
the correct paths and can validate them properly.
Change-Id: I2b3dbcfcfcf8a82e4e6df286cc3287aaa7fa2790
The current code checks to see that the destination path shares a prefix
with os.path.curdir. However, os.path.curdir is set to the directory
containing the playbook, not the root of the workdir, which means we're
not excluding things in the trusted dir like we'd like to be doing.
We already set HOME to the root of thew workdir, so we can just switch
the check from os.path.curdir to os.path.expanduser('~') and achieve the
original intent.
Change-Id: Ifac41f74f3306fe74b522c910867f9a5375bd61e
The normal action plugin was passing module_name when the code path
actually needed the module object itself.
Change-Id: Idb8968a7020ffb159d0d016b5bfab6bd88219c70
We greylist some modules in our action plugin blocking allowing them to
execute local code as long as it falls within safe constraints. Due to
the way ansible module loading works, a user could attack this by
creating a module in a local role or adjacent to a playbook that has the
same name as one of the modules we allow limited local execution. If
they did that it would allow them to execute arbitrary python code on
the executor.
Find the path of the module that will be executed in these cases and if
it is not within the ansible.modules package, disallow it. There are no
circumstances in which this is ok.
Change-Id: I7499e6b1091d745984ca36179de2793827c9f98f
Some of the lookup plugins access files on the executor host. Obviously
that's not what we want, so block them like we block action plugins.
password.py is banned, although it could be filtered. However, the
upstream code is fairly intense and slated for refactoring - so let's
wait until someone gets upset about it.
Change-Id: I6260b4658619a972b588c8bfba40ec33557bf2f6
People can use symlinks because they can. Expand them. Also, don't block
relative paths because we're expanding to absolute.
Change-Id: I483b5abbbeb962761d604dc5e7d6b64492dfd83d
Ansible imports our module with the name ansible.plugins.action.normal
(regardless of its actual path). This line would then import the original
module, also under the same name, so it would overwrite ours. Correct
this by importing the original with a unique name. It's not actually
used anywhere anyway.
Change-Id: I5dd26e2f89399f991cebc23839a36e7aff47056c
There are actions undertaken by action plugins in normal ansible that
allow for executing code on the host that ansible is executing on. We do
not want to allow that for untrusted code, so add a set of action
plugins that override the upstream ones and simply return errors.
Additionally, we can trap for attempts to execute local commands in the
normal action plugin by looking at remote_addr, connection and
delegate_to.
Change-Id: I57dbe5648a9dc6ec9147c8698ad46c4fa1326e5a
Clark Boylan