-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for libvips #1200
Comments
Thanks for the issue! I was just reviewing the rails changelog and working on getting Rails 7 alpha docs up when I noticed this. Extra contextWe've also have some related internal tickets (reference only, non-herokai won't be able to see them): Short term workaroundHere's a thread of people trying to get libvips working on heroku janko/image_processing#32 (comment). As a short term solution: I was able to get it working via the
We do have an "active storage previews" buildapck but I'm pretty sure it doesn't install libvips https://devcenter.heroku.com/articles/active-storage-on-heroku Long term solutionI'm going to bring up adding libvips to our default image. If you're curious, our stack image definitions are also open source https://github.com/heroku/stack-images. |
Great to hear - I'll have a play with the Would be awesome to see it in the default image (with the HEIF support too). Thanks! Edit: doesn't look like that
|
@dwightwatson sorry I missed your response. Can you open a support ticket with access to your app. We can debug the aptfile configuration there with more tools. https://help.heroku.com/ search for "libvips" and open a ticket. Link back to this comment and it will end up in my queue. For this specific issue it looks like The package you really need though is |
I opened a ticket (1030923) but I don't think it ended up assigned to you. We were able to debug that heroku-20/Ubuntu 20.04 installs libvips 8.9.1, when the current version is 8.11.3. Unfortunately there appears to be an issue prior to version 8.11.1 which prevents libvips from working with HEIF. HEIF support is pretty important as iPhones have used it as the default photo format for a while. Ideally these newer versions could be baked into the buildpack? |
I know it's hot off the press (Rails 7.0.0 has just been tagged), but I wonder if there's been any progress getting |
Hey @schneems don't mean to be a bother but wondering if there's any progress internally about this? Would be super helpful to those of us leveraging ActiveStorage to know whether this will actually happen. |
Hello, I'm one of the libvips maintainers. Another large plus for building your own libvips binary is security. The libvips that comes with Debian (for example) enables all possible loaders, and some of those load libraries have not been fuzzed. I expect they could be exploited easily, and this would obviously be bad for your customers. libvips has been in oss-fuss for a couple of years now: https://github.com/google/oss-fuzz/tree/master/projects/libvips I suggest you only enable that set of loaders. They should be pretty safe for untrusted input. In fact I'd go even further and also omit JXL for now. Fuzzing is still finding issues in libjxl relatively frequently. |
@jcupitt that's great to know. I don't suppose you'd have any guidance on building libvips on Heroku? |
@dwightwatson sorry, I use ruby and heroku a bit, but I'm not an expert on slugs and all that stuff. I was partly involved in https://elements.heroku.com/buildpacks/brandoncc/heroku-buildpack-vips , I don't know if that's helpful. |
To come back to this thread. We looked into adding libvips to heroku-18 or heroku-20 and don't feel comfortable doing that. The plan is to try to roll it into heroku-22. The numbers correspond with Ubuntu LTS releases so heroku-18 is based on Ubuntu 18. Our stack rollout doesn't come out the exact same time as the OS is released, but seeing the release date can let you know that it won't come out before then. Ubuntu is currently targeting April 21 https://linuxconfig.org/ubuntu-22-04-features-and-release-date. Our general policy is "we don't forecast features" so don't take this as a commitment, but read it as my personal intentions and desires. I'll try to keep this thread updated as we work through things. In the short term, I'm recommending those needing it to use the buildpack linked by @jcupitt. Thanks for the work there and for the recommendation! |
No problem @schneems, I'd like to see this happen too, so if there's anything I can do to help, please don't hesitate to ping me. There's a PR for 8.13 which adds a feature which might be relevant: a way to disable less-well tested operations at runtime: This would let rails use the platform libvips safely and not need to worry about building a custom binary. |
hi @jcupitt TL;DR: will heroku-buildpack-vips support Ruby 3.0.3 any time soon? LR: I ended up here after a somewhat rather lengthy detour 😓 for just wanting the latest/greatest in Apple Silicon, arm64 and what-not - and obviously libvips - and even though I've battled my way through/around quite a few issues along the road I fear this will be the end of it 😭 The "original" ruby buildpack was not enough so I did which now hands me this:
|
Sorry, I don't know, you'd need to ask Brandon. |
Unfortunately it seems that |
* Add libvips to stack images Also addresses heroku/heroku-buildpack-ruby#1200 GUS-W-9927197 * Add poppler-utils Since we now have libpoppler already. Very useful for the ActiveStorage-Preview buildpack. GUS-W-12613616
Thank you for your patience, everybody.
Please bear in mind that rollouts of stack image updates to Private Spaces take additional time and will happen over the next few days, so if your app runs in a Private Space, check with a |
Hi @dzuelke, this is great! I noticed a possible problem though, looking at the changelog you have:
If these libraries were added for libvips' benefit, I'm not sure this is a good idea. They have not been fuzzed (as far as I know) and are probably trivially hackable. I know at least With this set of libraries, any rails application which lets activestorage see untrusted data will almost certainly be vulnerable to remote execution attacks (or at the very least denial of service attacks) with crafted files. I would very strongly urge you to only build libvips with image format libraries that we've fuzzed. You can see our fuzzing set up here: https://github.com/libvips/libvips/blob/master/fuzz/oss_fuzz_build.sh I would also pick pdfium over poppler (poppler is GPL), but of course that's your call. I would also add cgif and remove libgif7, if libgif was put in for libvips to use. libvips has it's own GIF loader, but uses cgif for GIF write. It does not use libgif. |
Hi @jcupitt! Thanks for the fuzzing reminder. The trouble is that we are limited to I presume it's not possible to limit We're using Poppler since that, again, is supplied from upstream and receives security backports, unlike PDFium. The |
Hey @dzuelke, thank for the quick reply. Yes, I wondered if you were just using the ubuntu builds, that's a shame. libvips 8.13 and later has a thing to block untrusted loaders: https://www.libvips.org/2022/05/28/What's-new-in-8.13.html#blocking-of-unfuzzed-loaders It's off by default. I've suggested to janko/image_processing#105 (comment) Or perhaps heroku could set this by default somewhere? You'll know far more about this than me! This won't help older libvips installs, of course. |
Unfortunately, ubuntu-22 shipped with libvips 8.12, so I think none of these builds will have the thing to block untrusted loaders :( Perhaps libvips should be removed from heroku until the next LTS, which should support loader blocking? The alternative would be to build your own libvips binary with only the trusted loaders included (this is what the various buildpacks do), but that would be significantly more work for you, of course. Perhaps heroku could bless and help maintain an official libvips buildpack? That might be an easy compromise. |
Yeah, 8.12 unfortunately :( FWIW, having just the presence of We'll discuss a "libvips-fuzzed" buildpack internally. The most important thing for us is to link against as many libraries from the stack as possible, because our stack images underpin running dynos, meaning an update to packages affects all apps immediately thanks to dynamic linking. Having libraries supplied by a buildpack, on the other hand, means customers only get their packages updated when they redeploy their apps. Maybe you folks at libvips.org should provide a PPA with more recent builds for Ubuntu LTS versions? ;) |
Oh hmm I thought it was easy to unset env vars. Are there times when this is difficult?
Ooooof, we're really hoping to not do any more packaging :( It's so much work. I suppose I'm concerned that by shipping a libvips binary that's not suited for untrusted data, heroku might end up accidentally discouraging community packaging of more secure versions. That would be a very unfortunate outcome for everyone. |
Yup :) Imagine a Same on Heroku and any other platforms... one can set variables to override defaults a buildpack or the platform might set, but to remove that default value as a user, the "best" one can do is to make it an empty string. Since you y'all have released libvips with this behavior, it would be a bit risky to change it, since there might be people who just set the variable to an empty value (as opposed to "1") to enable the blocking, and suddenly, they'd "lose" the blocking with an update. Although if you clearly document it, or emit a warning now that it will change in the future if the value is empty... alternatively, have explicit "0", "no", "false" values to disable the blocking maybe?
Yup, we know, that's why we try to avoid it, too ;)
Yeah I know what you mean. I'm also looking through Rails' ActiveStorage and the Because if uploading an |
Rails 7.0.0 Alpha 1 has just been released, which makes a change to the default variant processor.
libvips
is now the default instead ofImageMagick
.While
ImageMagick
is still supported it seems reasonable that Heroku should support the default out of the box so that additional configuration isn't required to get a fresh Rails 7 app up and running. Furthermorelibvips
is a more performant option.Added info by maintainer, please don't change:
The text was updated successfully, but these errors were encountered: