Tracking Issue for Rust 2024: Make `std::env::{set_var, remove_var}` unsafe

[traviscross]

This is a tracking issue for making std::env::{set_var, remove_var} unsafe to call in Rust 2024.

• Add discussion that concurrent access to the environment is unsafe #116888 (comment)

About tracking issues

Tracking issues are used to record the overall progress of implementation. They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions. A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature. Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Steps

• Implement in nightly Rust 2024 for std::env::{set_var, remove_var}.
  • Make std::env::{set_var, remove_var} unsafe in edition 2024 #124636
• Implement migration lints for rustc_deprecated_safe_2024.
  • Make std::env::{set_var, remove_var} unsafe in edition 2024 #124636
  • Rename deprecated_safe lint to deprecated_safe_2024 #125990
  • Add TODO comment to unsafe env modification #126019
• Implement in nightly Rust 2024 for CommandExt::before_exec.
  • CommandExt::before_exec: deprecate safety in edition 2024 #125970
  • Allow to customize // TODO: comment for deprecated safe autofix #127857
• N/A: Add documentation to the dev guide.
  • See the instructions.
• N/A: Add documentation to the reference.
  • See the instructions.
• Add documentation to the edition guide.
  • Add 2024 unsafe functions edition-guide#304
• Ensure ready for Rust 2024 stabilization.

Unresolved Questions

TODO.

Related

• Make std::env::{set_var, remove_var} unsafe in edition 2024 #124636
• set_var/remove_var are unsound in combination with C code that reads the environment #27970
• Consider deprecating and/or modifying behavior of std::env::set_var #90308
• Don't trigger unsafe_op_in_unsafe_fn for deprecated safe fns #125925
• Remove lock guarantee from std::env::{set_var, remove_var} #125937
• CommandExt::before_exec: deprecate safety in edition 2024 #125970
• Rename deprecated_safe lint to deprecated_safe_2024 #125990
• Add TODO comment to unsafe env modification #126019
• Allow to customize // TODO: comment for deprecated safe autofix #127857

Implementation history

• Make std::env::{set_var, remove_var} unsafe in edition 2024 #124636

cc @tbu @Amanieu @rust-lang/libs-api

[traviscross]

@rustbot assign @tbu

[joboet]

I'm really not a big fan of this, because unsafe is completely unnecessary on well-behaved platforms like Windows, which provide a good API with proper synchronization, and on the platforms where this is an issue, we could easily make the API sound by properly enforcing that setenv is only called on the main thread (our current behaviour is technically unspecified, see MT-safe env). (not correct) it adds a very subtle safety condition that is almost impossible to reliable satisfy (quite some C library function access the environment internally and you'd have to guarantee that none of them is currently running).

I think the best solution would be to contribute new API to glibc/musl... that behaves correctly (e.g. by copying the GetEnvironmentVariable behaviour of copying into a user-provided buffer).

[BurntSushi]

@joboet The nooks and crannies of this issue have been discussed at length already. Your suggestion to "add a new API" to glibc/musl has been brought up before and it isn't viable. I would suggest at least skimming through that thread, and pay special attention to richfelker's comments if you think the solution to this problem can come from libc. (He is the maintainer of musl.)

I'm really not a big fan of this, because unsafe is completely unnecessary on well-behaved platforms like Windows, which provide a good API with proper synchronization

I don't think anyone is a big fan of this. The fact that Windows has a safe API for this isn't sufficient criteria for marking a platform independent API safe that is unsafe on some set of supported tier1 platforms. The answer here is to probably to publish a crate that exposes the safe Windows specific API.

[RalfJung]

I think the best solution would be to contribute new API to glibc/musl... that behaves correctly (e.g. by copying the GetEnvironmentVariable behaviour of copying into a user-provided buffer).

I agree. I hope someone will pursue this.

However, there was, as far as I can tell, basically zero progress on this in the almost 9 years (!) since #27970 was filed. I see no reason to believe that a fix for this will materialize medium-term (let's say, ≤ 5 years). Even on a 10-year scale I am not optimistic that anything will happen. Saying we have to wait until Linux and macOS get their APIs fixed is, at this point, basically equivalent to saying we're okay with this just not being fixed, and std being perpetually unsound when programs combine Rust code and C code.

Given that the best solution is just unrealistic, we should pursue the second-best solution, and that's what we are doing right now. I'm very happy to finally see progress on this.

it adds a very subtle safety condition that is almost impossible to reliable satisfy (quite some C library function access the environment internally and you'd have to guarantee that none of them is currently running).

The safety constraint is basically, don't mutate the environment if another thread may exist. That's the only way we can be sure there's no concurrently running C code. This is sad, but two of our Tier 1 OSes do not support anything better than that, and we can either accept or deny this fact but it remains a fact.

This hopefully pushes the Rust ecosystem towards APIs that avoid the need for mutating the environment.