cc @EliteTK since this code last changed in #17115

FWIW, I just verified the manual test from #17115 still works:

$ hdiutil create -size 1g -fs ExFAT -volname EXFATDISK exfat.dmg
$ hdiutil attach exfat.dmg
$ cd /Volumes/EXFATDISK
$ uv init --bare --cache-dir build/uv/cache -v

So test:extended doesn't seem run cargo test on macos... I'll fix that, but I'll just run those manually.

Pending that, it seems like this change maybe won't regress #11324 for uv, but will regress it for using pip with uv's cache, which I had no idea we supported...

Edit: Good news is, the tests pass on mac.

I'm a little confused as to why the /home/runner/.cache/uv/.lock file is the problem in those failing tests - all the uv invocations (aside from the initial venv) in that script pass --cache-dir=temp_dir.

Edit: I just noticed the uv pip show command (https://github.com/astral-sh/uv/blob/main/scripts/check_cache_compat.py#L43) does not specify --cache-dir, maybe that's part of the problem

It does seem like pip creates .lock files as 0o600 (https://github.com/pypa/pip/blob/794c144edc1a6fdf1090d1fa633acd9bde822639/src/pip/_vendor/cachecontrol/caches/file_cache.py#L22), which uv may have trouble with if running as a separate user, but probably not much to do about that?

I also ran these locally on my mac and couldn't reproduce, unless I manually set the lock to something like 0o400 owned by root.

The tests are failing when we're asking pip to use a cache directory that uv has created the lockfile in.

And the reason is probably this: https://github.com/tox-dev/filelock/blob/680b51c4b027b43bbf2d9bae2de45aebeebbdd03/src/filelock/_unix.py#L41

Does uv pip actually call out to pip? I saw but discounted the open flags in filelock because check_cache_compat just uses uv pip. But if that is indeed the problem, going with 644 or even 664 seems like a reasonable compromise.

Oh hold on, I didn't notice the uv in front of "pip" until now. You're right, we're not using pip with uv's cache...

That makes this much simpler to understand then.

Those tests are testing current uv vs previous uv. In the current uv we are only opening for reading, and only setting the read bits. In the previous version we were opening for reading and writing, and setting both bits.

So, if we stick to 644 (or just 666 and let the umask handle dropping the group/other write bits) we would actually be breaking backwards compatibility with older uv versions in the case described in #11324.

And, if we use 444, then we're just flat out breaking older uv versions accessing caches created by a newer uv.

The good news is, the upcoming 0.10.0 release will incorporate breaking changes. On the other hand, this may be a bit too breaking, not sure.

I'll sleep on it.

If we went with 644, the only breaking change would be when using an old version with a new lock file, and doing so as separate users. I can definitely see putting it off until 0.10.0, but it still seems like an edge case with an easy fix (update uv). Our current situation is we can't use uv because of "security" scans complaining about world-writable files, and we can't even set a umask as a workaround. Is it a silly thing? Probably. But it's also a bit of a smell that these files are world-writable when they don't have to be (imo).

Anyway, I really appreciate you taking the time to consider this! I'm pushing because I love the project and want to use it at work 😄

The easiest path forward is to ship this behind a preview feature flag, would it unblock your adoption if you could change the behavior by setting an environment variable or configuration file option?

Yes, we could definitely work with that. In that case, is it worth considering a UV_LOCKFILE_MODE environment variable, and/or a tool.uv.lockfile-mode config that defaults to 0o666 to match current behavior?

Yes, we could definitely work with that. In that case, is it worth considering a UV_LOCKFILE_MODE environment variable, and/or a tool.uv.lockfile-mode config that defaults to 0o666 to match current behavior?

For the current behaviour (without this PR applied), it only makes sense for write and read bits to be set or unset in tandem. The lockfile is never written to or read from, but we won't be able to open it for locking without both bits set¹. Being able to configure the lockfile mode to 752 or 725 would be equivalent in that it would effectively work the same as 600. Moreover, it only makes sense for the lockfile's read and write permissions to be as permissive as a cache entry's read permissions. So, to avoid confusion, we'd probably want a cache-perms setting or something which lets you specify the permissions at the group, user and other level, rather than specific mode bits.

But, instead, we could already do something like this most likely without breaking anything:

let read_mode = 0o444 & !umask;
// Mirror the read permission into the write permissions
let write_mode = read_mode >> 1;
let desired_mode = read_mode | write_mode;

The reason why this probably won't break anything is that if the umask restricts reading for group or other, then cache entries written by a uv running with such a umask would already not work for other users. So creating a lockfile with less restrictive permissions would only delay the inevitable permission errors down the road (although I'd like to test this a bit further to make sure there isn't actually a new failure case).

With the above approach, we wouldn't need any new configuration, we could just use the umask to inform the lockfile mode. Furthermore, once the behaviour of using 0o444 perms over 0o666 perms got stabilised, this behaviour would continue to work (without needing to ignore the umask for the write permission bits any more).

Unfortunately for your case, it would still set 0o666 permissions for a 0o022 umask. But, as I understand it, adding and enabling the new behaviour preview feature should get you the behaviour you want. e.g.:

• umask of 0o022 would create cache entries with 0o644 and a lockfile with 0o444
• umask of 0o027 would create cache entries with 0o640 and a lockfile with 0o440

Do you think this kind of approach would be sufficient for your purposes? I completely open to the possibility that I missed something in my analysis, so let me know if you think there's still another justification for a separate lockfile mode configurable or maybe even the cache permission setting described above.

Regardless, the change I describe here should probably be a separate PR, and it should include adding some targetted tests to the test suite.

On the other hand, we would be happy to accept this PR with the changes to the lockfile creation mode behind a preview flag and unconditional removal of the .write(true) on the lockfile opening. I'd even suggest opening a new PR to just drop .write(true) so we can merge that sooner, as that's kind of the whole source of all this unfortunate compatibility pain.

I think what you're saying makes sense in regard to lock files for caches not needing to be more permissive than the cache files themselves. However, LockedFile is used in more contexts than just caching, so it would feel weird tying lock permissions generally to cache-perms specifically.

I'm also not convinced we want to create lock files as 0o444 necessarily. Internally to uv it would likely be fine, as we would open the file for reading if it exists and lock on it. But it would prevent overwriting (or opening in append mode) even by the owner, which would affect compatibility with pip/filelock (if that is actually desired). Maybe it's just a gut feeling that it could be a footgun, I'm not sure.

My preference would be to drop the .write(true) and gate the mode behind a preview flag or config or whatever - I just don't know how to do that 😁 I'm also less inclined to think try_set_permissions needs to go if there's a mode setting.

I think what you're saying makes sense in regard to lock files for caches not needing to be more permissive than the cache files themselves. However, LockedFile is used in more contexts than just caching, so it would feel weird tying lock permissions generally to cache-perms specifically.

Good point. But I think this just reinforces that maybe we shouldn't have a UV_LOCKFILE_MODE or similar and instead should just aim towards a situation where we can rely solely on umask instead.

But it would prevent overwriting (or opening in append mode) even by the owner, which would affect compatibility with pip/filelock (if that is actually desired). Maybe it's just a gut feeling that it could be a footgun, I'm not sure.

Sorry, you can ignore the thing about pip and filelock, I got very confused because I didn't see that we were running uv pip. Our cache format isn't compatible with pip, there's no concern with regard to 0o444 there.

The only concern is that users trying to manually rm -r the cache or rm a lock file would now get the warning from rm...

On the one hand, .git directories are already famous for this. On the other hand, maybe we could just request 0o666 or 0o664 with the umask applied to avoid causing any grief in the 90% of cases.

My preference would be to drop the .write(true) and gate the mode behind a preview flag or config or whatever - I just don't know how to do that 😁

Regarding how to add a preview flag, this is a recent PR which may help: #17464

But, it looks somewhat involved to get the flag to the right place... Feel free to discuss it with us.

One last option is also that we take over and build on top of your PR.

I'm also less inclined to think try_set_permissions needs to go if there's a mode setting.

try_set_permissions must stay for now in non-preview, to avoid breakage. But for preview and beyond, it should be dropped.

I feel like any mode setting we introduce should probably also respect umask.

On the other hand, maybe we could just request 0o666 or 0o664 with the umask applied to avoid causing any grief in the 90% of cases.

This sounds ideal to me!

But, it looks somewhat involved to get the flag to the right place... Feel free to discuss it with us.

At first glance, it seems like it would need to be passed in to every acquire call? I didn't see a clean way to get settings into uv-cache (and maybe elsewhere) without a lot of churn. During that same glance, I also realized I'm out of my depth 😁

One last option is also that we take over and build on top of your PR.

That would be amazing if it's on the table. I'm not opposed to giving it a go with some guidance, but it will almost certainly be faster for you all to take it over. Happy to stub it out if it would be helpful, but it feels like the actual changes to LockedFile::create will be pretty minimal in comparison to whatever else needs to be done.

I feel like any mode setting we introduce should probably also respect umask.

Funny, I feel the other way - that if I'm going to tell uv specifically what mode the lockfiles should be, I'd expect them to be that mode. But I do agree that not having a specific mode setting and letting umask do its thing is better.