lp://staging/~zyga/checkbox/path-bug

Branch merges

Propose for merging

No branches dependent on this one.

Merged into lp://staging/checkbox at revision 2516

Sylvain Pineau (community): Approve on 2013-11-20

Diff: 641 lines (+224/-66)

2 files modified

plainbox/plainbox/impl/ctrl.py (+120/-40)
plainbox/plainbox/impl/test_ctrl.py (+104/-26)

api

Related source package recipes

No recipes using this branch. (?)

Create packaging recipe

Related snap packages

Related bugs

Bug #1248894: plainbox needs a way to share private scripts within one provider namespace	Critical	Fix Released
Bug #1252396: jobs specifying user have incorrect PATH set, can't find scripts	High	Fix Released

Link a bug report

Related blueprints

2519. By Zygmunt Krynicki on 2013-11-18

plainbox:ctrl: fix copy/paste docstring

Signed-off-by: Zygmunt Krynicki <email address hidden>

2518. By Zygmunt Krynicki on 2013-11-18

plainbox:ctrl: use env trick with sudo controller

This patch corrects one of the many faces of the 'path bug' (executables
not being in PATH in certain situations).

One aspect of that bug is how that 'sudo -E' which was being used in the
sudo execution controller is especially sensitive to ignore PATH even if
-E is used. Instead it loads the "secure path" from system wide
configuration file. This feature would obviously erase our customized
PATH and thus jobs using sudo this way would fail to see nest_dir with
all the executables.

The fix is to bring the sudo controller in alignment with pkexec
controller that already uses env(1) to pass arbitrary environment
variables into a program executed with sudo (without the -E flag this
time).

Signed-off-by: Zygmunt Krynicki <email address hidden>

2517. By Zygmunt Krynicki on 2013-11-18

plainbox:ctrl: fix incorrect docstrings

This patch is seemingly useless as the docstring and associated
funcionality is being replaced by subsequent patch but for completeness
I'd like to properly document how the sudo execution controller used to
work.

Signed-off-by: Zygmunt Krynicki <email address hidden>

2516. By Zygmunt Krynicki on 2013-11-18

plainbox:ctrl: fix how nest_dir is inserted into PATH

This patch corrects one of the many faces of the 'path bug' (executables
not being in PATH in certain situations).

One aspect of that bug is how nest_dir (directory, that has symlinks to
executables we need in PATH) was being injected into PATH. Previously it
was being done in execute_job(). This fails for execution controllers
that pass environment using a trick in get_execution_command(), such as
pkexec and plainbox-trusted-launcher execution controllers. It fails
because those controllers need to pass "differential" environment inside
the command itself and the change occurs outside of
get_execution_environment().

The fix is straightforward, to move all PATH manipulations to happen
inside get_execution_environment() and adjust all tests that needed
adjusting.

Signed-off-by: Zygmunt Krynicki <email address hidden>

2515. By Zygmunt Krynicki on 2013-11-18

plainbox:ctrl: refactor nest directory handling

This patch changes how we construct the nest_dir and populate it with
symlinks to various executables. Instead of being a bit of code inside
execute_job() it is now a dedicated context manager,
configured_filesystem() that can be tested in isolation and, more
importantly, mocked away in other places.

Note that execute_job() still does mkdir() CHECKBOX_DATA but it is worth
considering moving all filesystem manipulations to the new context
manager later on.

Signed-off-by: Zygmunt Krynicki <email address hidden>

2514. By Zygmunt Krynicki on 2013-11-18

plainbox:ctrl: fix get_differential_execution_environment

This patch corrects one of the many faces of the 'path bug' (executables
not being in PATH in certain situations).

One aspect of that bug was how the "differential" environment, one that
was about to be passed to another process via the env(1) trick, was
being computed. Due to a simple mistake one of the cases where we wanted
to pass a modified environment variable was written incorrectly, thus
plainbox would not pass any changed but already existing variables
across privilege elevation mechanisms.

The fix is trivial, just compare the right variables.

Signed-off-by: Zygmunt Krynicki <email address hidden>

2513. By Zygmunt Krynicki on 2013-11-18

plainbox:ctrl: remove reference to extra_PATH

The extra_PATH attribute was removed from Provider1 class sometime ago,
there is no need to provide mocked values for it anymore.

Signed-off-by: Zygmunt Krynicki <email address hidden>

2512. By Zygmunt Krynicki on 2013-11-18

plainbox:ctrl: add tests for SymLinkNest class

Just missing tests for SymLinkNest class that was added last week.

Signed-off-by: Zygmunt Krynicki <email address hidden>

2511. By Zygmunt Krynicki on 2013-11-18

"automatic merge by tarmac [r=brendan-donegan][bug=][author=zkrynicki]"

2510. By Zygmunt Krynicki on 2013-11-16

Incremented changelog