-
Notifications
You must be signed in to change notification settings - Fork 902
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
Rework Toolchain model and drop relative file path overrides #3340
Conversation
dfc8730
to
d2e42c0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have noticed a few minor issues.
Looks great and much clearer. Thank you!
Relative path overrides permit a freshly downloaded source tree to execute arbitrary code on any rustup command that executes a binary from the configured toolchain, and its a reasonable tradeoff for us to remove this feature. Absolute path overrides are kept intact - these were added to support users of large monorepo tool systems, and can be kept with reasonable safety. Introduce an interior model for toolchain names which allows the elimination of conditional code for custom vs named vs path based toolchains. Fixes rust-lang#3130. Finishes the separation of Toolchain vs DistributableToolchain vs CustomToolchain started some time ago. Moves some test support into the src tree in order to keep symbols private and adds a 'test' feature required for self-testing, ensuring that that code doesn't affect production builds. Changes the CI debug builds to use the new test feature and also otel, so that otel doesn't bitrot.
I think relative paths are a subset of absolute paths, so, security-wise, this is a no-op. Namely, |
Verified with
on |
That's definitely unfortunate. I think removing path-based toolchains altogether is going to break things, especially for more corporate users with additional build orchestration around Cargo (e.g., Amazon). How feasible is it to disallow specifically |
@rbtcollins is the issue that absolute paths are also unsafe tracked anywhere? |
Not yet, please do open a bug from this, and we'll have to discuss whether to give up, filter some absolute paths and hope that's sufficient, or some other thing. |
Rework Toolchain model and drop relative file path overrides
Relative path overrides permit a freshly downloaded source tree to
execute arbitrary code on any rustup command that executes a binary from
the configured toolchain, and its a reasonable tradeoff for us to remove
this feature. Absolute path overrides are kept intact - these were added
to support users of large monorepo tool systems, and can be kept with
reasonable safety.
Introduce an interior model for toolchain names which allows the
elimination of conditional code for custom vs named vs path based
toolchains. Fixes #3130. Close #3174
Finishes the separation of Toolchain vs DistributableToolchain vs
CustomToolchain started some time ago.
Moves some test support into the src tree in order to keep symbols
private and adds a 'test' feature required for self-testing, ensuring
that that code doesn't affect production builds.
Adds in a new build that tests the combination of features together (on just one platform), to ensure all combinations are valid.