[ros2launch] Follow symlinks when walking. #345
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mikepurvis
requested review from
adityapande-1995 and
methylDragon
as code owners
Signed-off-by: Mike Purvis <mpurvis@clearpath.ai>
mikepurvis
force-pushed
the
walk-follow-symlinks
branch
from
966eade
to
804f163
Compare
mikepurvis
changed the title
|
We discussed this, and we are a little concerned about the security implications of following symlinked directories. This would seem to open users up to symlink swapping attacks. That said, we also see how this could be useful. So we think that we should leave the default to not follow symlinks, but add an option to |
|
Hmm. We disagree with the reasoning on this— what is the scenario where an adversary can change a symlink on you and you aren't already owned in a dozen other ways? In any case, if it's at least a config option then we can more trivially patch our version to change the default. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes an issue with using
ros2 launchinside symlink-joined workspaces, such as when constructing them using systems like Nix or Bazel. Really these loop should just be rewritten to usescandirinstead (which follows symlinks as the default behaviour), but this is a simpler change in the immediate.Similar change to ros/ros#225.
Credit for this find to @iwanders.