Bug 331795 - Amarok's build breaks sandboxing
Summary: Amarok's build breaks sandboxing
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: Tools/Script Manager (show other bugs)
Version: 2.8-git
Platform: Gentoo Packages Linux
: NOR grave
Target Milestone: 2.9
Assignee: Amarok Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-06 03:54 UTC by Darin McBride
Modified: 2014-11-10 00:53 UTC (History)
6 users (show)

See Also:
Latest Commit:
Version Fixed In: 2.9


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Darin McBride 2014-03-06 03:54:37 UTC
Gentoo's build process has a sandboxing feature to ensure build processes don't touch parts of the system in ways that can't be tracked by the build process to ensure proper cleanup on removal as well as to help detect file collisions and the such.  Until recently, Amarok's build process worked fine under this environment.

After trying to rebuild it, I found sandbox errors popping up, which, under Gentoo's system, causes the whole build to fail.  Presumably, this will cause problems for other distributions as well as they likely try to install each component into a temporary directory so they can package it up via RPM, deb, or whatever, as well, and any files that get written directly to /usr are likely to be missed.

Since I was using the git repository here, I managed to bisect it down to the commit fd23d814d3a03a4a73809c63780191f016a8bc7b, which is where the lines:

set(args ${CMAKE_SOURCE_DIR}/src/scriptengine/  ${DATA_INSTALL_DIR}/amarok/scriptconsole)
#shouldn't be here
execute_process(COMMAND python ${CMAKE_SOURCE_DIR}/src/scriptengine/PHAACG2.py ${args})

were added.  It's actually in the execute_process here that we're encountering the sandbox violation as it seems ${args} points, at least by default, to /usr/local/share/apps/amarok/scriptconsole.  When building with a target of, for example, /var/tmp/portage/media-sound/amarok-9999/distdir, this needs to be taken into account so that you only touch files either in the current build output directory or the redirected destination directory (which I think would be /var/tmp/portage/media-sound/amarok-9999/distdir/usr/local/share/apps/amarok/scriptconsole)

Given that this is happening pretty much immediately during the initial cmake, the build directory makes the most sense.  A separate install-time target should copy the files to their specified destination.

Thanks,

Reproducible: Always
Comment 1 Myriam Schweingruber 2014-03-06 07:15:26 UTC
Subscribing release manager and devs
Comment 2 Anmol Ahuja 2014-03-06 09:33:46 UTC
My fault. As the comment says, that shouldn't really be in the cmake: it's generating a pseudoheader for doxygen which isn't really relevant to the build process, and a string list for auto-completion in the console (it's not dynamic). So should I bundle a list of strings with the script console or skip the auto-complete?
Comment 3 Myriam Schweingruber 2014-03-06 16:36:48 UTC
I presume this is not the only place where we use something like this, did you  dig the codebase? ack-grep is your friend for that...
Comment 4 Darin McBride 2014-03-06 17:24:01 UTC
If I go back to a commit prior to the one I labelled above, I don't get this sandbox violation.  Therefore, up to that point, there was no other place where amarok used something like this.

If there are places after this, I'm not sure, but it's fairly trivial for me to test any fix simply by trying to re-emerge (which automatically grabs the latest from git) to see if there are more.  Probably more trivial than the effort required to make just the fix as per above :)
Comment 5 Myriam Schweingruber 2014-03-06 22:01:12 UTC
Darin, that was not addressed to you, but for Anmol. He should check in Amarok on how to solve the issue, not you , as it is his commit that breaks sandboxing :) I am pretty sure we have code in Amarok that does what he wants to achieve, but solved properly.
Comment 6 Darin McBride 2014-03-06 22:33:00 UTC
On Thursday March 6 2014 10:01:12 PM you wrote:
> https://bugs.kde.org/show_bug.cgi?id=331795
> 
> --- Comment #5 from Myriam Schweingruber <myriam@kde.org> ---
> Darin, that was not addressed to you, but for Anmol. He should check in
> Amarok on how to solve the issue, not you , as it is his commit that breaks
> sandboxing
> :) I am pretty sure we have code in Amarok that does what he wants to
> :achieve,
> but solved properly.

Fair enough.  I misunderstood where you were actually pointing Anmol to look 
for other parts of Amarok for a solution.  I thought you were concerned about 
having the same problem elsewhere, which, given that there have been a bunch 
of commits since, could still be a problem introduced again elsewhere.

In that case, I am willing to retry the git image once it's fixed to be sure 
all cases were found and resolved, not just the first.

I am a dev, too, just not a kde dev.  I switched from C++ to perl many many 
years ago :) but I can help out, especially in ways that scratch my itches, 
such as rebuilding under a sandbox.  I already do that as often as I remember, 
just to get new features/fixes/bugs ( :D ), it just turns out that this is 
unfortunately timed as I'm trying to deploy a brand new box, and amarok is 
what I'd call a "critical application" - I can't work without my music :)

And thus why I was trying to harass even a newbie in IRC to fixing this - I'm 
being as patiently impatient as I can given that, without amarok, it really is 
the end of the world (ok, that's still figurative, but you get the idea) :D
Comment 7 Anmol Ahuja 2014-03-07 01:44:03 UTC
(In reply to comment #3)
> I presume this is not the only place where we use something like this, did
> you  dig the codebase? ack-grep is your friend for that...

Okay. I won't have time today,  I'll check it tomorrow. But all I want to know if is it's acceptable bundling a bunch of strings with the code? I think it feels ugly- maybe I should skip this and try for dynamic autocompletion.
Comment 8 Anmol Ahuja 2014-03-07 01:50:32 UTC
> having the same problem elsewhere, which, given that there have been a bunch 
> of commits since, could still be a problem introduced again elsewhere.

It's not.

> In that case, I am willing to retry the git image once it's fixed to be sure 
> all cases were found and resolved, not just the first.

It's minor, just comment out the line in cmake for now, it won't break anything.

> just to get new features/fixes/bugs ( :D ), it just turns out that this is 
> unfortunately timed as I'm trying to deploy a brand new box, and amarok is 
> what I'd call a "critical application" - I can't work without my music :)

Understandably :)
Sorry about this.
Comment 9 Till Schäfer 2014-07-14 12:13:36 UTC
this bug is still valid. (small reminder ping, as this blocks testing for another amarok bug)
Comment 10 Dan Meltzer 2014-11-09 16:10:14 UTC
Till, did a609db3bbfd215f043876590be677625198be5a7 fix this? From 07/23.  Looks like it should have...
Comment 11 Till Schäfer 2014-11-09 18:15:21 UTC
this is fixed for me in current git. thx
Comment 12 Myriam Schweingruber 2014-11-10 00:53:50 UTC
(In reply to Till Schäfer from comment #11)
> this is fixed for me in current git. thx

Closing with link to the commit