Shards' `postinstall` considered harmful

Personally I’d say that it would mean that we have integration between the dependency manager and the various build systems that can be used for a shard, but if you think that makes it a build system, then by all means, then I want shards to be a build system. Because everything else is directly user hostile.

As for it being a very bad idea like you think, in Rust with Cargo each crate knows how to build itself and support build scripts. If it was a bad idea, then people would complain about it or want to remove that particular feature. Do they? No! Cargo is generally liked as THE way to build crates.

I suspect the people who have a problem with how Rust and Cargo do things aren’t spending a lot of time in the Rust community complaining but instead just went elsewhere.

Having the build system execute the dependency manager makes the most sense for mixed-language projects. And while I can see the convenience case for just having shards execute the build system (could just be a script) in pure Crystal projects, I feel like it should probably be an extra step or parameter instead of the default.

As for Ameba, I have never used the version some shards install in “bin/”. I have a copy installed under “/usr/local/bin” that I occasionally use. Installing binaries in project directories that are not project-specific just feels weird.

4 Likes

Actually shards is already a build system + dependency manager, otherwise the shards build command and the targets: key wouldn’t be available.

I agree the ameba use case is irritating, but when this can be solved using --without-development as said above if you are only taking a look in some shards that uses it.

2 Likes

How long does it take to build ameba, and how many times shards install is ran for the first time, where this postinstall script happens?

I understand that this takes time, but reducing this initial time compared to the benefits it can bring, I’m not sure it’s worth it.

1 Like

IMO postinstall simplifies a lot things when the shard need some extra steps like compiling a tool or compiling a C library.

Shard authors can make the postinstall point to a script that depending on environment variables build or not something.

I’m thinking that there’s a single postinstall but it’s used for different things:

  1. compiling a related C library so that the shard works. Without doing this the shard simply doesn’t work!
  2. providing an executable as a tool. This is optional and depends on each user.

It seems what’s annoying here is point 2, where an executable is built but the user doesn’t need it.

Would it make sense to split postinstall into different hooks? I’m not sure which ones… for instance, the ameba binary is more of a developer tool used by direct shards, but indirect shards might not need it.

1 Like

I would love to get rid of the postinstall stuff on Lucky, but I just don’t see an easy path. We could just make none of the tasks precompiled, which might make like 1 or 2 people happy from the shards install side of things, but maybe 10 people less happy from the tool usage side.

Just to better explain our usage, some of the shards used within Lucky come with a CLI generator tool. These just generate files which never change, so there’s no reason to recompile on each run. We do have a way that you can compile these after the fact, but it compiles ALL of the tasks include the db.migrate task. This one you don’t want precompiled because each time you generate a new migration, it has to re-compile those generated files.

It seems like the real issue here is just a documentation issue. If shard authors understand that calling shards build from postinstall will also install their shard’s development dependencies, then they can ensure they add the right --without-development flag. Maybe a section on the website guides for “Best practices for authoring a shard”?

3 Likes

Noting this is being discussed again, I agree with @straight-shoota that this is a potential OSi vulnerability (Operating System Injection) and RCE (Remote Code Execution)

This feature is an opt-out and maybe should be instead opt-in? there is a long Twitter discussion about it at: https://twitter.com/bararchy/status/1549821323030962176

2 Likes

“long Twitter discussion”? I see only a handful of tweets…

There’s little point in removing postinstall for security unless macro shell commands are also removed.

ameba isn’t always used.
lucky commands are probably used by all/most lucky projects. They should at least be available to avoid wtf’s and “why isn’t this working” support requests.

If (2) is split in the following manner I think there would be fewer complaints.

2a. Providing an executable when directly required by a shard (opt-in) (ameba)
2b. Providing an executable useful to all projects regardless of shard requirement hierarchy (lucky commands)

That’s true. But at least it’s a start.
And security considerations are not the only reason for removing it.
Removing the need to wait for postinstall hooks to build and install potentially unnecessary executables is an improvement to user experience.
And IMO the biggest point is portability. Most postinstall scripts expect a POSIX environment. If you install those shards on Windows, they’ll fail. Even if the shard should work perfectly fine on Windows. So it’s just functionally insufficient.

1 Like

This can easily be handled by a simple shell script which delegates to the wrapped CLI or if unavailable either prints instructions on how to get it or just builds it on demand. So things will just continue to keep working.
According to the Lucky docs, you can install the Lucky CLI globally in your path instead of compiling it for each project. So postinstall isn’t even necessary for that.

1 Like

To add my own opinion to the mix, I agree that postinstall is too big of a hammer as is in the sense you can run any command to do anything with very few guidelines, but having this automation step built into shards is a huge win overall. I’d be fine replacing this with a similar mechanism that compiles crystal code (say), but trying to remove it in place of shims or manual build steps needing to be run by the developer would be a step backwards in user experience.

What I like most about the postinstall building binaries today is that when I run shards update, it also updates my binaries automatically if any of those shards saw changes, but doesn’t rebuild if no updates to the shard happened. I feel a shim might miss this, and manual steps absolutely would. Same with having a single global binary installed under /usr/bin or /bin, I find they all quickly fall behind latest that way.

I might have missed it, but how does ameba (or other binaries) get installed without the developer wanting it? Doesn’t it need to be explicitly added to shards.yml to have the post install script run?

2 Likes

A shim could totally take care of that. Ideally, it would just invoke make (or whatever build system the shard is using) so you get an implicit rebuild when the source has changed.

Yes, it needs to be explicitly added. But even if it’s added in the repository, it’s not always explicitly needed. For example, when I just want to check out the shard code, I don’t need ameba installed. Even if I do a simple change, I might not need to run ameba to verify it (assuming CI would fail on the odd chance it produces a lint warning).

Forget POSIX, some shards post-install stuff doesn’t even work unless it’s glibc and bash!

Running shards install with --skip-postinstall and --skip-executables is pretty much the default for me due to developing mostly for Alpine Linux Docker containers.

Having a short version of those would be nice when manually testing things out in Alpine as it’s a bit lengthy to type.
A platform-independent way for people to do their post-install stuff would also be great.

1 Like

Removing the need to wait for postinstall hooks to build and install potentially unnecessary executables is an improvement to user experience.

But forcing the user to manually invoke commands to build subdependencies is a pain in the ass from a user experience point of view. So it is definitely not a one-sided question.

There was a suggestion in the twitter thread to give the user a question of what to do with the postinstall hooks. That could be a start.

And IMO the biggest point is portability. Most postinstall scripts expect a POSIX environment.

Yes. The answer to this is to expand shards into a build system which can support different actions on different systems.

Even if the shard should work perfectly fine on Windows.

I may not be representative, but none of the C bindings I’ve done should work fine, because they are directly binding against Linux specific stuff.

So it’s just functionally insufficient.

Yes, and the answer is a build system that handles it, or at least gives the proper tools to automate it. Shards is currently not sufficient in this area, and hence people use postinstall because there is no other way to solve the problems.

2 Likes

I didn’t read the entire thread, but… are there past incidences of this, either for Crystal or other languages that allow such things?

A bunch of things are just c libraries that compile just as well on Linux/libc, Linux/musl, BSD, Windows, etc… But the shard post-install only accounts for Linux/libc.


So obviously people don’t like using Make, likely because it’s not that user-friendly.

One solution is to turn shards into a full build system. This is likely not going to work that great with multi-language projects.

Another solution would be to pick a “blessed” build system like Make/Just/Rake or something else and then provide some great docs/tutorials/education on how to use it with Crystal.

Third option. Create a new modern build system that focuses on user experience. This would be a large effort, but it’s possible to make something that works great and handles multi-language projects well.

Fourth. Remove post-install from shards and let each project figure it out on its own.

Fifth. Leave it as is.

…I am sure that there are more possible solutions that I haven’t thought of.
Currently, I feel like the second option is the better one.

5 Likes

I actually like to simply call rake as the postinstall command, so I can add support for further platforms without any hassle (I am also just not a fan of make, it does some things in a overly complicated way).

However this obviously requires Ruby for installing the shard, so a Crystal-based solution might be the better way (maybe even using crystal i in the future?).

The security concerns above are absolutely valid, though, as even calling a Makefile is a potential hazard. Sadly I have no real idea how to solve this - and I don’t think this problem is limited to Crystal (npm and rubygems have similar concerns resulting in massive problems). But dropping a postinstall option without replacement is also not optimal, since this will most likely damage the ecosystem massively.

Difficult topic :smiley:

3 Likes