-
Notifications
You must be signed in to change notification settings - Fork 13
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
Default cache directory should be user-specific #35
Comments
Thanks for the report! Yes permissions in The advice I give currently is that users should configure The generic issue aside, I agree this specific failure behavior isn't great. I have a TODO to isolate the cache directory per-user, which would make this experience a bit better. We can also improve this error message to at least be clearer. One thought is that we perhaps should only lock down the cache directory if not specified by the user (i.e. via Another approach would be to add a flag to configure the permissions the directory is created with. That's much more explicit, which I like, but it's still cumbersome. Also, just to point out, these permissions are (currently?) only set if the directory doesn't exist, so a workaround is to simply use your own directory that you've already created with your desired permissions. I'm not convinced ignoring the permission issue in this case is "correct" behavior (it just seems wasteful to check the directory permissions every time), so don't assume this workaround will continue to work indefinitely. |
Another thing to note - sharing a cache directory across multiple users can be a problem even if you intended to do so (i.e. you're not concerned about security). Because the cache doesn't key off the process' invoking user you can get cached output polluted with state from the other user. As a simple example, caching your user ID:
So I'd strongly recommend using separate directories per-user even if you don't think it's necessary from a security standpoint - correctness could also be impacted. I will likely resolve that TODO in a future release so that this happens by default. |
believe me there's many places where the output for any user and root are the same, here's an example: i'm using it could as well be used in cgi web pages that take lot of time to build page, as well as |
Oh totally, I appreciate that it's fine in many cases. But in the general case it's both a security risk and a correctness risk - and the correctness risks can be subtle - so I'm pretty hesitant to intentionally support cross-user caching (at least not without requiring something explicit like both callers specifying the same |
Wouldn't |
Yes possibly, but it would have a performance impact on systems that use a tmpfs for |
For an actual performance impact, there would have to be lots of accesses, of which most likely would be reads. So the default file-system cache (assuming there's enough available RAM for that) of the operating system likely will speed this up also without an underlying tmpfs. |
Agreed, I'm just sharing the original motivation for FWIW it's pretty easy to observe a meaningful performance difference between caching to a tmpfs vs. local disk. I am also aware of users that rely on In the meantime, anyone wanting to use |
Thanks for sharing this background; I know that these tradeoffs are hard, and it's good that you care about backwards compatibility! |
Why do we need to move it out of /tmp to get per-user directories? I also ran into the same issue as @alexmyczko, so I added the following to my shell rc files:
So the cache dir is now Is there a reason that shouldn't be the default? |
Oh we don't, I was just responding to the suggestion to use Also, the distinction is relatively minimal, but I'd suggest setting |
so i really like bkt alot, however on multiuser system. or when root is using bkt -- somecommand, and after wards user tries bkt -- somesamecommandasother user ending up with above msg :'(
strace revelead to me:
not sure if this is a build or runtime option? probably i'm using it wrong and there's options. but would prefer to have it just work without options (thinking)
The text was updated successfully, but these errors were encountered: