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

Resolve path issues with cp.push #35413

Merged
merged 5 commits into from
Aug 16, 2016
Merged

Conversation

cachedout
Copy link
Contributor

@cachedout cachedout commented Aug 12, 2016

What does this PR do?

This PR modifies the master file recv method to take a list instead of a string for a path name for incoming files. This approach easier cross-platform compatibility concerns by allowing the master to reconstruct the path for whatever platform it happens to be on, regardless of what path separator and other idiosyncrasies the minion might have.

This resolves a bug wherein Windows minions would often write out nonsense or otherwise incorrect file paths to the master cache dir.

Additionally, it also implements additional safety features to ensure that a malicious actor can not write to files outside of the master's cache directory.

What issues does this PR fix or reference?

#35296

Previous Behavior

Windows minions would construct paths with backslashes in their name in the master cache directory.

New Behavior

Paths constructed correctly.

Tests written?

Yes

Please review Salt's Contributing Guide for best practices.

Mike Place added 4 commits August 12, 2016 18:33
To support multiple platforms, it will be difficult to try to account
for various path seperators and drive lettering schemes on the receiving
end of a file push. Instead, transition to an interface wherein the file
path is first split and seperators removed prior to it being sent from
the minion to the master.
@cro
Copy link
Contributor

cro commented Aug 13, 2016

Go Go Jenkins!

@cro
Copy link
Contributor

cro commented Aug 13, 2016

@cachedout Looks good to me.

@s0undt3ch
Copy link
Collaborator

Looks great! I'd just have a test which triggers salt's security routines ;)

load_path_normal = os.path.normpath(load_path)

# If this is Windows and a drive letter is present, remove it
load_path_split_drive = os.path.splitdrive(load_path_normal)[1:]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does this end up being used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guh. Thanks. Fixing.

@thatch45 thatch45 merged commit 240fc12 into saltstack:2015.8 Aug 16, 2016
@Ch3LL Ch3LL mentioned this pull request Dec 9, 2016
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.

5 participants