Opened 7 years ago

Closed 5 years ago

#16736 closed defect (fixed)

sagenb: SandboxViolation in webassets

Reported by: vbraun Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: packages: standard Keywords:
Cc: ppurka, leif, rws, fbissey Merged in:
Authors: Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by vbraun)

Processing webassets-0.9.tar.gz
Writing /tmp/easy_install-hklIUJ/webassets-0.9/setup.cfg
Running webassets-0.9/setup.py -q bdist_egg --dist-dir /tmp/easy_install-hklIUJ/webassets-0.9/egg-dist-tmp-Cwkw_W
error: SandboxViolation: chmod('/home/buildslave-sage/slave/sage_git/dot_sage/.python-eggs/Pillow-2.2.2-py2.7-linux-x86_64.egg-tmp/PIL/tmpelb5a7.$extract', 493) {}

Change History (51)

comment:1 Changed 7 years ago by vbraun

The reason is that webasset setup.py imports sphinx to do some crap. Which ultimately imports PIL/Pillow which causes the egg to be unpacked, violating the sandbox.

Why does sagenb come with webassets to start with? Its not used anywhere, it seems. Some packaged js files were generated with it?

comment:2 Changed 7 years ago by vbraun

  • Branch set to u/vbraun/sagenb__sandboxviolation_in_webassets

comment:3 Changed 7 years ago by vbraun

  • Authors set to Volker Braun
  • Commit set to a487a0d673527ab9a9b57c36edf9c5ed0a7ac483
  • Description modified (diff)
  • Status changed from new to needs_review

I guess the ideal solution would be to use distutils / pip instead of easy_install (who cares about binary eggs)? Though that would probably be a bigger task. For now, just allow chmod outside of the setuptools sandbox (and, really, fixing permissions isn't so bad).

Updating to the latest setuptools alone didn't fix it, but we should do that anyway.


New commits:

425ebccUpdate to setuptools 5.4.1
a487a0dAllow chmod outside of the sandbox

comment:4 Changed 7 years ago by vbraun

  • Priority changed from major to blocker

comment:5 Changed 7 years ago by vbraun

  • Cc leif rws fbissey added

comment:6 Changed 7 years ago by fbissey

Not sure what webassets does either. Is it just me or there is a violent inflation in setuptools version numbers - worse than chrome even.

comment:7 Changed 7 years ago by fbissey

In fact I hadn't noticed that webassets had been upgraded past 0.7.1.

comment:8 Changed 7 years ago by vbraun

The whole packages-inside-sagenb thing needs to be fixed at one point, too. But this is also not the scope of this ticket, but make a new beta that can be installed from scratch.

comment:9 Changed 7 years ago by vbraun

This seems to be caused by #16733, so I'll unmerge that upgrade until somebody can review this.

comment:10 Changed 7 years ago by vbraun

  • Component changed from user interface to packages: standard
  • Priority changed from blocker to major

comment:11 Changed 7 years ago by fbissey

I am not convinced by the solution I will try something in the next few hours before accepting this.

comment:12 Changed 7 years ago by fbissey

So far I haven't been able to reproduce this. I am guessing this is a from scratch install while I am working on top of beta7 which may have an impact.

comment:13 Changed 7 years ago by vbraun

You probably need to (re-)install by hand the new docutils and pillow. Or rebuild from scratch (with #16733 merged in).

comment:14 Changed 7 years ago by strogdon

This is very curious. I merged #16733 and this ticket w/o the sandbox.patch (the patch was removed and the change was committed with git commit -a). The following sequence did not give a SandboxViolation in webassets:

  • ./sage -f docutils
  • ./sage -f setuptools
  • ./sage -f sagenb

But the following sequence did yield the violation (new docutils installed)

  • ./sage -f pillow
  • ./sage -f setuptools
  • ./sage -f sagenb

Then adding the sandbox patch in and doing

  • ./sage -f setuptools
  • ./sage -f sagenb

seemed to resolve things. So where is the real problem? Maybe it doesn't matter. The sandbox patch does work.

comment:15 Changed 7 years ago by fbissey

Interesting digging Steve. I am sure the patch works, I just think it is the wrong thing to do but I don't know any alternative, apart from trying to upgrade webassets since there is a newer release - of course it is likely it will suffer the same fate.

comment:16 Changed 7 years ago by vbraun

I tried upgrading webassets and it didn't work. The setup.py is till importing sphinx, so it'll die if that ends up pulling in pil(low)

comment:17 Changed 7 years ago by fbissey

OK so that potentially pushes the problem in sphinx and upgrading that brings its own problems. Or may be pillow, that would be easier to try that.

comment:18 Changed 7 years ago by strogdon

I've installed sphinx-1.2.2, merged #16733 and this ticket w/o the sandbox patch and I have no SandboxViolation. However, #16396 can't be merged w/o some work. Is it possible that sphinx-1.2.2 doesn't import pillow? I believe what I've done is correct. But maybe the sandbox patch is the thing to do in the short-term.

comment:19 Changed 7 years ago by fbissey

I really think that shooting an element of the sandbox for just one package is not good. It would be better if an exception could be made until we fix the faulty package rather than sabotaging setuptools.

comment:20 Changed 7 years ago by fbissey

OK the call to sphinx in webassets is really limited to

try:
    from sphinx.setup_command import BuildDoc
    cmdclass = {'build_sphinx': BuildDoc}
except ImportError:
    cmdclass = {}

at the top of setup.py There are calls in docs/conf.py but they probably won't be called if we remove the above. I propose we have a patched webassets in sagenb rather than breaking setuptools' sandbox.

comment:21 Changed 7 years ago by strogdon

Perhaps naive but is there any undesirable side effect of adding something like

export PYTHON_EGG_CACHE="./.python-eggs"

to build/pkgs/sagenb/spkg-install? I'm not sure where "." is during the install of sagenb but it's apparently not outside the sandbox.

comment:22 follow-up: Changed 7 years ago by strogdon

Doing the above, a build from scratch was successful w/o the sandbox patch.

comment:23 in reply to: ↑ 22 Changed 7 years ago by fbissey

Replying to strogdon:

Doing the above, a build from scratch was successful w/o the sandbox patch.

I definitely would take that over the current patch. It is much simpler.

comment:24 Changed 7 years ago by vbraun

Putting the egg cache in a location that you aren't sure of is potentially a security issue.

I've asked on sage-devel whether we can remove webassets: https://groups.google.com/d/msg/sage-devel/fEeGgVWuk7I/Nmc5kcVXfZYJ

comment:25 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:26 Changed 7 years ago by vbraun

Any news? If you want a different solution then post a branch...

comment:27 Changed 7 years ago by fbissey

I think the best fix is to push for #16396 to be finished.

comment:28 Changed 6 years ago by kcrisman

Just to note that the reason for webassets was discovered, though hopefully this is a moot point now that #16396 is positive review.

comment:29 Changed 6 years ago by fbissey

I very much hope we can fix it invalid once the whole new docutils/sphinx/sagenb is in.

comment:30 Changed 6 years ago by strogdon

If things are fixed then there is perhaps the question of whether to upgrade setuptools anyway. But maybe another ticket.

comment:31 Changed 6 years ago by strogdon

OK, I've merged #16396 and #16733 on top of 6.4.beta6 and I don't see the SandboxViolation when building sagenb. I would feel more comfortable if someone else would also confirm this. I also tried things with setuptool-5.4.1 and when rebuilding sphinx-1.2.2 (after a make) I get an error

Removing old version of Sphinx...
rm: cannot remove '/64bitdev/storage/sage-git_develop/sage/local/lib/python/site-packages/Sphinx-1.2.2-py2.7.egg/sphinx': Directo
ry not empty
Error building Sphinx: 'Error deleting previous version'

The only file in the subject folder is

ls /64bitdev/storage/sage-git_develop/sage/local/lib/python/site-packages/Sphinx-1.2.2-py2.7.egg/sphinx
__init__.pyc

comment:32 Changed 6 years ago by strogdon

Well I'm unable to reproduce the above sphinx build error. I'm not sure what the source could have been. Also I see no SandboxViolation with setuptools-5.4.1 added on top of #16396 and #16733 without the sandbox.patch.

comment:33 Changed 6 years ago by fbissey

  • Branch u/vbraun/sagenb__sandboxviolation_in_webassets deleted
  • Commit a487a0d673527ab9a9b57c36edf9c5ed0a7ac483 deleted
  • Reviewers set to Steve Trogdon
  • Status changed from needs_review to positive_review

A bit late. I just noticed we haven't updated docutils in 6.4.rc0/rc1 because this ticket is blocking it. This is a positive review "works for me/invalid" I guess.

comment:34 Changed 6 years ago by vbraun

  • Milestone changed from sage-6.4 to sage-duplicate/invalid/wontfix
  • Resolution set to invalid
  • Status changed from positive_review to closed

You are supposed to set the milestone to invalid...

comment:35 Changed 6 years ago by vbraun

FWCW, I just triggered it again while building Sage in a subdirectory of /tmp:

Processing webassets-0.10.1.tar.gz
Writing /tmp/easy_install-OSzZkb/webassets-0.10.1/setup.cfg
Running webassets-0.10.1/setup.py -q bdist_egg --dist-dir /tmp/easy_install-OSzZkb/webassets-0.10.1/egg-dist-tmp-H58hNo
error: SandboxViolation: chmod('/home/vbraun/.sage/.python-eggs/Pillow-2.2.2-py2.7-linux-x86_64.egg-tmp/PIL/tmpMF0VYS.$extract', 493) {}

comment:36 Changed 6 years ago by strogdon

This is curious. I did a fresh build under /tmp after cloning from the develop branch and I get on my gentoo box:

Processing webassets-0.10.1.tar.gz
Writing /tmp/easy_install-KBbcXK/webassets-0.10.1/setup.cfg
Running webassets-0.10.1/setup.py -q bdist_egg --dist-dir /tmp/easy_install-KBbcXK/webassets-0.10.1/egg-dist-tmp-7cFRfA
warning: no files found matching 'run_tests.py'
no previously-included directories found matching 'docs/_build'
warning: no previously-included files matching 'out.css' found under directory 'examples'
warning: no previously-included files matching 'out.js' found under directory 'examples'
no previously-included directories found matching 'examples/appengine-sdk'
warning: no previously-included files matching '*.pyc' found anywhere in distribution
warning: no previously-included files matching '.gitignore' found anywhere in distribution
warning: no previously-included files matching '*.orig' found anywhere in distribution
warning: no previously-included files matching 'webassets-cache/*' found anywhere in distribution
warning: no previously-included files matching '.sass-cache/*' found anywhere in distribution
zip_safe flag not set; analyzing archive contents...
webassets.filter.__init__: module references __file__
Adding webassets 0.10.1 to easy-install.pth file
Installing webassets script to /tmp/sage_sandbox/sage/local/bin

Installed /tmp/sage_sandbox/sage/local/lib/python2.7/site-packages/webassets-0.10.1-py2.7.egg
Processing dependencies for webassets==0.10.1
Finished processing dependencies for webassets==0.10.1

I wonder what's up?

comment:37 Changed 6 years ago by fbissey

OK guys that prompt 2 questions from me:

  • Is it only happening when building in /tmp ?
  • /tmp/easy_install-${random}? is it really the spkg being installed or easy_install acting on its own?

comment:38 Changed 6 years ago by vbraun

Only happens when building in /tmp. Seems that easy_install does something different in that case.

comment:39 Changed 6 years ago by vbraun

  • Milestone changed from sage-duplicate/invalid/wontfix to sage-6.5
  • Resolution invalid deleted
  • Status changed from closed to new

I'm getting this again with sagenb-11.4 on some but not all user accounts. No idea what the difference is. This is in my home directory, not compiling in /tmp.

Processing webassets-0.10.1.tar.gz
Writing /tmp/easy_install-AR1rXi/webassets-0.10.1/setup.cfg
Running webassets-0.10.1/setup.py -q bdist_egg --dist-dir /tmp/easy_install-AR1rXi/webassets-0.10.1/egg-dist-tmp-dpb_Mg
error: SandboxViolation: chmod('/home/vbraun/.sage/.python-eggs/Pillow-2.2.2-py2.7-linux-x86_64.egg-tmp/PIL/tmpZhc7i8.$extract', 493) {}

How about we just get rid of the unused webassets?

comment:40 Changed 6 years ago by vbraun

PS: the directory is created by setuptools just for webassets:

$ grep AR1rXi logs/pkgs/sagenb.log 
Writing /tmp/easy_install-AR1rXi/webassets-0.10.1/setup.cfg
Running webassets-0.10.1/setup.py -q bdist_egg --dist-dir /tmp/easy_install-AR1rXi/webassets-0.10.1/egg-dist-tmp-dpb_Mg

comment:41 Changed 6 years ago by fbissey

Yes get rid of it, especially if it is such a headache. Is the problem happening on some distros and not others or something more subtle?

comment:42 Changed 6 years ago by vbraun

A different account can install webassets (again in its respective home directory):

Processing webassets-0.10.1.tar.gz
Writing /tmp/easy_install-Fd5CnH/webassets-0.10.1/setup.cfg
Running webassets-0.10.1/setup.py -q bdist_egg --dist-dir /tmp/easy_install-Fd5CnH/webassets-0.10.1/egg-dist-tmp-f2qRbU
warning: no files found matching 'run_tests.py'
[...]

comment:43 Changed 6 years ago by vbraun

  • Description modified (diff)
  • Reviewers Steve Trogdon deleted

comment:44 Changed 6 years ago by kcrisman

Again, webassets is there so that people can use the "newui" branch of sagenb, which apparently people are indeed trying out and using (based on discussions there).

comment:45 Changed 6 years ago by strogdon

Just off the top of my head. Would it be possible to avoid the issue by removing webassets from sagenb and then installing it separately?

comment:46 follow-up: Changed 6 years ago by vbraun

If you need webassets you can just "pip install webassets". But if its not used by default then don't ship it.

comment:47 in reply to: ↑ 46 Changed 6 years ago by kcrisman

If you need webassets you can just "pip install webassets". But if its not used by default then don't ship it.

I'd be open to a pull request, but whatever solution there is should probably have a good and easy way to tell a user to install webassets before they attempt to switch to that. It's also possible migeruhito's changes with that will have removed the need for webassets, I really know nothing about it beyond why it was added (included not knowing why that was actually necessary).

comment:48 Changed 6 years ago by vbraun

Then just add it to the instructions for trying out the newui branch, wherever those might be.

comment:49 Changed 5 years ago by jdemeyer

  • Authors Volker Braun deleted
  • Milestone changed from sage-6.5 to sage-duplicate/invalid/wontfix
  • Reviewers set to Volker Braun
  • Status changed from new to needs_review

webassets is being removed at #14840.

comment:50 Changed 5 years ago by jdemeyer

  • Status changed from needs_review to positive_review

comment:51 Changed 5 years ago by vbraun

  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.