Skip to content
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

Tell rustc wrappers which envs to pass through to allow env sandboxing #14444

Closed
fenollp opened this issue Aug 22, 2024 · 6 comments
Closed

Tell rustc wrappers which envs to pass through to allow env sandboxing #14444

fenollp opened this issue Aug 22, 2024 · 6 comments
Labels
A-build-execution Area: anything dealing with executing the compiler A-build-scripts Area: build.rs scripts A-configuration Area: cargo config files and env vars C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request.

Comments

@fenollp
Copy link

fenollp commented Aug 22, 2024

Problem

In the process of creating a $RUSTC_WRAPPER I realize it needs to know which environment variables build scripts and crates read in order to pass them down to my calls to rustc (and not pass other, possibly sensitive, envs).

These envs can be set in config as the rustc-env map cc https://doc.rust-lang.org/cargo/reference/config.html

These are also set when executing build scripts that output cargo::rustc-env=VAR=VALUE directives cc https://doc.rust-lang.org/cargo/reference/build-scripts.html#rustc-env

Proposed Solution

A simple solution could be for cargo to set an env when calling rustc (or just when calling a $RUSTC_WRAPPER).
This env only needs to contain environment variable names (these are already set in env by cargo).

I propose to set CARGO_BUILD_SETS_ENVS to a list of space-separated env names (or =-separated, both are illegal in var names).

E.g. compiling the crate self_update would set CARGO_BUILD_SETS_ENVS=HOST_PLATFORM TARGET_PLATFORM
cf https://github.com/Shnatsel/current_platform/blob/57c123569e12f55c2111046f97294750f12c467d/src/build.rs

Notes

I propose .._SETS_ENVS as I'd also like .._READS_ENVS for names corresponding to cargo::rerun-if-env-changed=NAME but this feature request requires further exploration on my part and a whole new discussion.

There are probably other information that my $RUSTC_WRAPPER should have access to (e.g. LD_PRELOADed things, ...). Maybe you have opinions, ideas on this. I'm just not done exploring yet.

@fenollp fenollp added C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-triage Status: This issue is waiting on initial triage. labels Aug 22, 2024
@epage epage changed the title Tell rustc which envs to pass through Tell rustc wrappers which envs to pass through to allow env sandboxing Aug 22, 2024
@epage epage added A-configuration Area: cargo config files and env vars A-build-execution Area: anything dealing with executing the compiler A-build-scripts Area: build.rs scripts S-needs-team-input Status: Needs input from team on whether/how to proceed. and removed S-triage Status: This issue is waiting on initial triage. labels Aug 22, 2024
@fenollp
Copy link
Author

fenollp commented Aug 22, 2024

Related: #5282

@weihanglo
Copy link
Member

@weihanglo
Copy link
Member

I feel like Cargo should not reinvent --env-set in inself, and should probably just wait for the stabilization of the feature.

@weihanglo
Copy link
Member

While --env-set is an approach to fulfill this feature, it might be considered as a breaking change to recursive cargo calls, like what has been described in #14194. This kind of change may need a new edition to make it the default behavior.

@ehuss
Copy link
Contributor

ehuss commented Oct 22, 2024

The cargo team took a look at this issue, but we were unclear about exactly what the use case is, or how all the pieces relate to one another. Can you start from a high level to explain what you are trying to accomplish, what kind of sandboxing you are trying to build, what kind of threat models there are, etc? We were also unclear, since things like env! mean that you don't know which environment variables rustc will be reading until after it reads them.

@ehuss ehuss added S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request. and removed S-needs-team-input Status: Needs input from team on whether/how to proceed. labels Oct 22, 2024
@fenollp
Copy link
Author

fenollp commented Oct 23, 2024

This is probably enough: #14444 (comment)

My issue can be closed I believe.

@fenollp fenollp closed this as completed Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-execution Area: anything dealing with executing the compiler A-build-scripts Area: build.rs scripts A-configuration Area: cargo config files and env vars C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request.
Projects
None yet
Development

No branches or pull requests

4 participants