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

twoliter: disallow std::path::Path::canonicalize #431

Merged
merged 1 commit into from
Jan 3, 2025

Conversation

cbgbt
Copy link
Contributor

@cbgbt cbgbt commented Jan 3, 2025

Issue number:

Closes #427

Description of changes:
Twoliter uses the location of Twoliter.toml to locate the rest of the Twoliter workspace. Prior to this change, it used std::fs::canonicalize to do so. This function resolves symlinks on the filesystem, which is a problem when you're building in the context of a symlink farm.

This change moves to using path-absolutize for such canoncicalization, which uses unix separator and path traversal semantics to "canonicalize" a path without resolving symlinks or actually consulting with the host's filesystem.

The change also adds clippy lints to attempt to forbid future use of canonicalization functions that can cause this to regress.

See #427 for more details.

Testing done:

  • New integration tests pass
  • Created a twoliter workspace where Twoliter.toml was a symlink to another workspace. Issuing a build on the old version caused build artifacts to be placed in the build directory behind the symlink. With the new change, build artifacts were placed in the new workspace as expected.

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@sumukhballal
Copy link
Contributor

Nice!

@cbgbt cbgbt merged commit a9323d8 into bottlerocket-os:develop Jan 3, 2025
1 check passed
@cbgbt cbgbt deleted the canonicalize branch January 3, 2025 22:44
@mikn
Copy link

mikn commented Jan 24, 2025

You were so quick closing this out I didn't even notice! Thank you @cbgbt ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use of fs::canonicalize makes Twoliter difficult to wrap in other build systems
4 participants