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.

Sign up for GitHub

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

Jump to bottom

Deny setting CARGO_HOME in [env] table #11590

Closed
ehuss opened this issue Jan 17, 2023 · 0 comments · Fixed by #11644 or rust-lang/rust#107778
Closed

Deny setting CARGO_HOME in [env] table #11590

ehuss opened this issue Jan 17, 2023 · 0 comments · Fixed by #11644 or rust-lang/rust#107778
Labels
A-configuration Area: cargo config files and env vars A-environment-variables Area: environment variables

Comments

@ehuss
Copy link
Contributor

ehuss commented Jan 17, 2023

After some discussion (and tying into #10094 and #11589), we should consider denying setting CARGO_HOME in [env] now. Cargo itself does not change its home directory based on that value. Recursive calls to cargo will use that new value. We suspect that the likelyhood of someone already doing this is low, and we'd like to close off the possibility sooner rather than later. We think it is the wrong behavior to only pass it to recursive invocations, we'd like more control over this, particularly until we decide the semantics of #11589.
@ehuss ehuss added A-configuration Area: cargo config files and env vars A-environment-variables Area: environment variables labels Jan 17, 2023
@basile-henry basile-henry mentioned this issue Jan 28, 2023
Merged
bors added a commit that referenced this issue Feb 1, 2023
@bors
Auto merge of #11644 - basile-henry:basile-henry/deny-cargo-home-env-…
cf410cb 
…table, r=ehuss

config: Deny CARGO_HOME in [env] table (fixes #11590)

Fixes #11590
@bors bors closed this as completed in d09e132 Feb 1, 2023
@weihanglo weihanglo mentioned this issue Feb 7, 2023
Merged
weihanglo added a commit to weihanglo/rust that referenced this issue Feb 7, 2023
@weihanglo
Update cargo
6f38fd5 
12 commits in e84a7928d93a31f284b497c214a2ece69b4d7719..82c3bb79e3a19a5164e33819ef81bfc2c984bc56
2023-01-31 22:18:09 +0000 to 2023-02-04 22:52:16 +0000

- util toml targets: Do not infer directory as a file (rust-lang/cargo#11678)
- Add more guidance on how to implement unstable features (rust-lang/cargo#11675)
- Fix unstable chapter layout for codegen-backend (rust-lang/cargo#11676)
- refactor: mod.rs over "name".rs for consistency (rust-lang/cargo#11673)
- Verify source before recompile (rust-lang/cargo#11672)
- doc: more doc comments and intra-doc links (rust-lang/cargo#11669)
- Turn off debuginfo for build dependencies v2 (rust-lang/cargo#11252)
- config: Deny CARGO_HOME in [env] table (fixes rust-lang/cargo#11590) (rust-lang/cargo#11644)
- Fix the wrong comment (rust-lang/cargo#11651)
- Add partial support for SSH known hosts markers (rust-lang/cargo#11635)
- Replace `winapi` with `windows-sys` crate. (rust-lang/cargo#11656)
- Handle .cargo-ok being truncated (rust-lang/cargo#11665)
bors added a commit to rust-lang-ci/rust that referenced this issue Feb 8, 2023
@bors
Auto merge of rust-lang#107778 - weihanglo:update-cargo, r=weihanglo
7ba4e95 
Update cargo

12 commits in e84a7928d93a31f284b497c214a2ece69b4d7719..82c3bb79e3a19a5164e33819ef81bfc2c984bc56 2023-01-31 22:18:09 +0000 to 2023-02-04 22:52:16 +0000

- util toml targets: Do not infer directory as a file (rust-lang/cargo#11678)
- Add more guidance on how to implement unstable features (rust-lang/cargo#11675)
- Fix unstable chapter layout for codegen-backend (rust-lang/cargo#11676)
- refactor: mod.rs over "name".rs for consistency (rust-lang/cargo#11673)
- Verify source before recompile (rust-lang/cargo#11672)
- doc: more doc comments and intra-doc links (rust-lang/cargo#11669)
- Turn off debuginfo for build dependencies v2 (rust-lang/cargo#11252)
- config: Deny CARGO_HOME in [env] table (fixes rust-lang/cargo#11590) (rust-lang/cargo#11644)
- Fix the wrong comment (rust-lang/cargo#11651)
- Add partial support for SSH known hosts markers (rust-lang/cargo#11635)
- Replace `winapi` with `windows-sys` crate. (rust-lang/cargo#11656)
- Handle .cargo-ok being truncated (rust-lang/cargo#11665)

r? `@ghost`
RalfJung pushed a commit to RalfJung/miri that referenced this issue Feb 9, 2023
@bors
Auto merge of #107778 - weihanglo:update-cargo, r=weihanglo
385ea96 
Update cargo

12 commits in e84a7928d93a31f284b497c214a2ece69b4d7719..82c3bb79e3a19a5164e33819ef81bfc2c984bc56 2023-01-31 22:18:09 +0000 to 2023-02-04 22:52:16 +0000

- util toml targets: Do not infer directory as a file (rust-lang/cargo#11678)
- Add more guidance on how to implement unstable features (rust-lang/cargo#11675)
- Fix unstable chapter layout for codegen-backend (rust-lang/cargo#11676)
- refactor: mod.rs over "name".rs for consistency (rust-lang/cargo#11673)
- Verify source before recompile (rust-lang/cargo#11672)
- doc: more doc comments and intra-doc links (rust-lang/cargo#11669)
- Turn off debuginfo for build dependencies v2 (rust-lang/cargo#11252)
- config: Deny CARGO_HOME in [env] table (fixes rust-lang/cargo#11590) (rust-lang/cargo#11644)
- Fix the wrong comment (rust-lang/cargo#11651)
- Add partial support for SSH known hosts markers (rust-lang/cargo#11635)
- Replace `winapi` with `windows-sys` crate. (rust-lang/cargo#11656)
- Handle .cargo-ok being truncated (rust-lang/cargo#11665)

r? `@ghost`
@ehuss ehuss mentioned this issue May 7, 2023
Merged
bors added a commit that referenced this issue May 7, 2023
@bors
Auto merge of #12101 - ehuss:disallow-rustup-home, r=weihanglo
1490d10 
Disallow RUSTUP_HOME in the [env] table.

This adds a check to prevent RUSTUP_HOME from being set in the `[env]` config table under the same reasoning as was done in #11590. Cargo will likely behave incorrectly if this key is set in the config since it will not change the home used by the outer cargo itself.

This is a breaking change, though I think it is unlikely to be used in practice. When cargo is executed via a rustup proxy, the proxy sets RUSTUP_HOME which overrides the `[env]` table entry. It may be feasible that someone is invoking cargo directly without the rustup wrappers, and then using this to steer the rustc invocations to a different rustup location, but I'm not sure that's a use case we need to be supporting.

This is being added as a further assurance for #11590 to make sure the environment is configured as expected.

We could potentially add other Rustup env vars to reject, but I'm not sure I want to delve into analyzing all the possible reasons or interactions for each one.
@ehuss ehuss mentioned this issue May 8, 2023
Merged
bors added a commit that referenced this issue May 8, 2023
@bors
Auto merge of #12107 - ehuss:disallow-rustup-toolchain, r=weihanglo
1efb562 
Disallow RUSTUP_TOOLCHAIN in the [env] table.

This adds RUSTUP_TOOLCHAIN to the list of disallowed keys in the `[env]` table. This is intended to help prevent confusion and broken environments, along with the same reasoning as #12101 and #11590.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-configuration Area: cargo config files and env vars A-environment-variables Area: environment variables
Projects
None yet
Milestone
No milestone
1 participant
@ehuss